[PATCH V2 0/6] regulator: enable regulator support for i.MX23/i.MX28

[PATCH V2 0/6] regulator: enable regulator support for i.MX23/i.MX28

[Stefan Wahren]

This patch series adds on-chip regulator support to Freescale i.MX23 and i.MX28.
It's the result of the last discussion about cpufreq-dt support [1]. This series
doesn't contain cpufreq-dt support anymore because it violated a constraint
between VDDD voltage and EMI clock frequency (Thanks to Juergen Borleis 
for pointing out).

The cpufreq-dt support would be a further patch series.

Summary
=======

Patch 1,2,3 - enable syscon support and add driver for mxs power subsystem
Patch 4,5,6 - enable regulator support for i.MX23/i.MX28

The main use of mxs_power driver is to trigger probing of the underlying DT
child nodes like the on-chip regulators. The mxs-regulator driver provides
for instance the voltage scaling support.

A detailed description of the i.MX28 power subsystem is in the reference manual 
[2] and application note 4199 [3].

Changes since V1
================
- rebase on Linux 4.1-rc1
- remove cpufreq-dt support
- use syscon for register access as suggested by Mark Brown
- rework regulator ramp up handling as suggested by Mark Brown
- use real switching frequencies for DC-DC instead of clock settings
- add warning in case of invalid switching frequency as suggested by Mark Brown
- some code clean ups
- add ASCII art of regulators to binding doc
- add more comments to regulator driver
- add COMPILE_TEST to dependencies for mxs power as suggested by Sebastian Reichel
- make use of stmp_device.h

[1] - http://comments.gmane.org/gmane.linux.power-management.general/58287
[2] - http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf
[3] - http://cache.freescale.com/files/32bit/doc/app_note/AN4199.pdf

Stefan Wahren (6):
  DT: add binding for mxs power subsystem
  ARM: mxs: select syscon for platform
  power: mxs_power: add driver for mxs power subsystem
  DT: add binding for mxs on-chip regulators
  regulator: add mxs on-chip regulator driver
  ARM: dts: enable regulator support for i.MX23/i.MX28

 .../devicetree/bindings/power/mxs_power.txt        |   17 +
 .../bindings/regulator/mxs-regulator.txt           |   65 +++
 arch/arm/boot/dts/imx23.dtsi                       |   40 +-
 arch/arm/boot/dts/imx28.dtsi                       |   40 +-
 arch/arm/mach-mxs/Kconfig                          |    1 +
 drivers/power/Kconfig                              |    8 +
 drivers/power/Makefile                             |    1 +
 drivers/power/mxs_power.c                          |  136 +++++
 drivers/regulator/Kconfig                          |    8 +
 drivers/regulator/Makefile                         |    1 +
 drivers/regulator/mxs-regulator.c                  |  540 ++++++++++++++++++++
 11 files changed, 855 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/mxs_power.txt
 create mode 100644 Documentation/devicetree/bindings/regulator/mxs-regulator.txt
 create mode 100644 drivers/power/mxs_power.c
 create mode 100644 drivers/regulator/mxs-regulator.c

-- 
1.7.9.5

[PATCH V2 1/6] DT: add binding for mxs power subsystem

[Stefan Wahren]

This patch adds the device tree bindings for the Freescale MXS
on-chip power subsystem.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../devicetree/bindings/power/mxs_power.txt        |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/mxs_power.txt

diff --git a/Documentation/devicetree/bindings/power/mxs_power.txt b/Documentation/devicetree/bindings/power/mxs_power.txt
new file mode 100644
index 0000000..4003ef7
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/mxs_power.txt
@@ -0,0 +1,17 @@
+Power subsystem for Freescale i.MX23/i.MX28
+
+Required properties:
+- compatible: Should be "fsl,<soc>-power". The supported SoCs include
+  imx23 and imx28.
+- reg: Address and length of the register set for the power subsystem.
+
+Optional properties;
+- interrupts: Interrupts used by the power subsystem
+
+Example for i.MX28:
+
+	power: power@80044000 {
+		compatible = "fsl,imx28-power", "syscon";
+		reg = <0x80044000 0x2000>;
+		interrupts = <6>;
+	}
-- 
1.7.9.5

[PATCH V2 2/6] ARM: mxs: select syscon for platform

[Stefan Wahren]

Select syscon for the MXS platform because it's required by the
power subsystem.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 arch/arm/mach-mxs/Kconfig |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-mxs/Kconfig b/arch/arm/mach-mxs/Kconfig
index 8479413..bf04d38 100644
--- a/arch/arm/mach-mxs/Kconfig
+++ b/arch/arm/mach-mxs/Kconfig
@@ -17,6 +17,7 @@ config ARCH_MXS
 	depends on ARCH_MULTI_V5
 	select ARCH_REQUIRE_GPIOLIB
 	select CLKSRC_MMIO
+	select MFD_SYSCON
 	select PINCTRL
 	select SOC_BUS
 	select SOC_IMX23
-- 
1.7.9.5

[PATCH V2 3/6] power: mxs_power: add driver for mxs power subsystem

[Stefan Wahren]

This patch adds a minimal driver for the Freescale i.MX23, i.MX28
power subsystem. It's required to trigger the probing of the underlying
drivers like on-chip regulators.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/power/Kconfig     |    8 +++
 drivers/power/Makefile    |    1 +
 drivers/power/mxs_power.c |  136 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 145 insertions(+)
 create mode 100644 drivers/power/mxs_power.c

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 4091fb0..0fcc158 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -43,6 +43,14 @@ config MAX8925_POWER
 	  Say Y here to enable support for the battery charger in the Maxim
 	  MAX8925 PMIC.
 
+config MXS_POWER
+	tristate "Freescale MXS power subsystem support"
+	depends on ARCH_MXS || COMPILE_TEST
+	help
+	  Say Y here to enable support for the Freescale i.MX23/i.MX28
+	  power subsystem. This is a requirement to get access to on-chip
+	  regulators, battery charger and many more.
+
 config WM831X_BACKUP
 	tristate "WM831X backup battery charger support"
 	depends on MFD_WM831X
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index b7b0181..8edcad7 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_GENERIC_ADC_BATTERY)	+= generic-adc-battery.o
 obj-$(CONFIG_PDA_POWER)		+= pda_power.o
 obj-$(CONFIG_APM_POWER)		+= apm_power.o
 obj-$(CONFIG_MAX8925_POWER)	+= max8925_power.o
+obj-$(CONFIG_MXS_POWER)		+= mxs_power.o
 obj-$(CONFIG_WM831X_BACKUP)	+= wm831x_backup.o
 obj-$(CONFIG_WM831X_POWER)	+= wm831x_power.o
 obj-$(CONFIG_WM8350_POWER)	+= wm8350_power.o
diff --git a/drivers/power/mxs_power.c b/drivers/power/mxs_power.c
new file mode 100644
index 0000000..669bfb1
--- /dev/null
+++ b/drivers/power/mxs_power.c
@@ -0,0 +1,136 @@
+/*
+ * Freescale MXS power subsystem
+ *
+ * Copyright (C) 2014 Stefan Wahren
+ *
+ * Inspired by imx-bootlets
+ */
+
+/*
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/stmp_device.h>
+#include <linux/types.h>
+
+#define BM_POWER_CTRL_POLARITY_VBUSVALID	BIT(5)
+#define BM_POWER_CTRL_VBUSVALID_IRQ		BIT(4)
+#define BM_POWER_CTRL_ENIRQ_VBUS_VALID		BIT(3)
+
+#define HW_POWER_5VCTRL_OFFSET	0x10
+
+#define BM_POWER_5VCTRL_VBUSVALID_THRESH	(7 << 8)
+#define BM_POWER_5VCTRL_PWDN_5VBRNOUT		BIT(7)
+#define BM_POWER_5VCTRL_ENABLE_LINREG_ILIMIT	BIT(6)
+#define BM_POWER_5VCTRL_VBUSVALID_5VDETECT	BIT(4)
+
+#define HW_POWER_5VCTRL_VBUSVALID_THRESH_4_40V	(5 << 8)
+
+struct mxs_power_data {
+	void __iomem *base_addr;
+	struct power_supply *ac;
+};
+
+static enum power_supply_property mxs_power_ac_props[] = {
+	POWER_SUPPLY_PROP_ONLINE,
+};
+
+static int mxs_power_ac_get_property(struct power_supply *psy,
+				     enum power_supply_property psp,
+				     union power_supply_propval *val)
+{
+	int ret = 0;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_ONLINE:
+		val->intval = 1;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+	return ret;
+}
+
+static const struct of_device_id of_mxs_power_match[] = {
+	{ .compatible = "fsl,imx23-power" },
+	{ .compatible = "fsl,imx28-power" },
+	{ /* end */ }
+};
+MODULE_DEVICE_TABLE(of, of_mxs_power_match);
+
+static const struct power_supply_desc ac_desc = {
+	.properties	= mxs_power_ac_props,
+	.num_properties	= ARRAY_SIZE(mxs_power_ac_props),
+	.get_property	= mxs_power_ac_get_property,
+	.name		= "ac",
+	.type		= POWER_SUPPLY_TYPE_MAINS,
+};
+
+static int mxs_power_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct resource *res;
+	struct mxs_power_data *data;
+	struct power_supply_config psy_cfg = {};
+	void __iomem *v5ctrl_addr;
+
+	if (!np) {
+		dev_err(dev, "missing device tree\n");
+		return -EINVAL;
+	}
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	data->base_addr = devm_ioremap_resource(dev, res);
+	if (IS_ERR(data->base_addr))
+		return PTR_ERR(data->base_addr);
+
+	psy_cfg.drv_data = data;
+
+	data->ac = devm_power_supply_register(dev, &ac_desc, &psy_cfg);
+	if (IS_ERR(data->ac))
+		return PTR_ERR(data->ac);
+
+	platform_set_drvdata(pdev, data);
+
+	v5ctrl_addr = data->base_addr + HW_POWER_5VCTRL_OFFSET;
+
+	/* Make sure the current limit of the linregs are disabled. */
+	writel(BM_POWER_5VCTRL_ENABLE_LINREG_ILIMIT,
+	       v5ctrl_addr + STMP_OFFSET_REG_CLR);
+
+	return of_platform_populate(np, NULL, NULL, dev);
+}
+
+static struct platform_driver mxs_power_driver = {
+	.driver = {
+		.name	= "mxs_power",
+		.of_match_table = of_mxs_power_match,
+	},
+	.probe	= mxs_power_probe,
+};
+
+module_platform_driver(mxs_power_driver);
+
+MODULE_AUTHOR("Stefan Wahren <stefan.wahren@i2se.com>");
+MODULE_DESCRIPTION("Freescale MXS power subsystem");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5

Re: [PATCH V2 3/6] power: mxs_power: add driver for mxs power subsystem

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 588 bytes --]

Hi,

On Wed, Apr 29, 2015 at 10:32:24PM +0000, Stefan Wahren wrote:
> This patch adds a minimal driver for the Freescale i.MX23, i.MX28
> power subsystem. It's required to trigger the probing of the underlying
> drivers like on-chip regulators.

I'm not sure what you need the power supply class for. I see only
one property (ac connected), which is constant. Maybe just drop
the power supply stuff?

This would leave an almost empty driver, which just sets a single
register at probe time and probes more drivers. That would probably
be more fitting for the MFD subsystem.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH V2 3/6] power: mxs_power: add driver for mxs power subsystem

[Stefan Wahren]

Hi Sebastian,

Am 23.05.2015 um 19:41 schrieb Sebastian Reichel:
> Hi,
>
> On Wed, Apr 29, 2015 at 10:32:24PM +0000, Stefan Wahren wrote:
>> This patch adds a minimal driver for the Freescale i.MX23, i.MX28
>> power subsystem. It's required to trigger the probing of the underlying
>> drivers like on-chip regulators.
> I'm not sure what you need the power supply class for. I see only
> one property (ac connected), which is constant. Maybe just drop
> the power supply stuff?

those SoCs could be powered by 3 different sources:
- 5V wall
- USB
- battery

I choose the first one as a base, because it's the simplest one.
Unfortunately i don't have the time to implement all of them.

The Freescale downstream kernel (based on 2.6.35) [1] has a driver to
handle all MXS power sources. But i think a clean implementation from
the scratch would be necessary for mainline.

> This would leave an almost empty driver, which just sets a single
> register at probe time and probes more drivers. That would probably
> be more fitting for the MFD subsystem.

I don't have any problems with fitting the driver to MFD.

> -- Sebastian

[1] -
http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/drivers/power/mxs/linux.c?h=imx_2.6.35_maintain

Re: [PATCH V2 3/6] power: mxs_power: add driver for mxs power subsystem

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1788 bytes --]

Hi Stefan,

On Wed, May 27, 2015 at 09:12:48AM +0200, Stefan Wahren wrote:
> Am 23.05.2015 um 19:41 schrieb Sebastian Reichel:
> > On Wed, Apr 29, 2015 at 10:32:24PM +0000, Stefan Wahren wrote:
> >> This patch adds a minimal driver for the Freescale i.MX23, i.MX28
> >> power subsystem. It's required to trigger the probing of the underlying
> >> drivers like on-chip regulators.
> > I'm not sure what you need the power supply class for. I see only
> > one property (ac connected), which is constant. Maybe just drop
> > the power supply stuff?
> 
> those SoCs could be powered by 3 different sources:
> - 5V wall
> - USB
> - battery
> 
> I choose the first one as a base, because it's the simplest one.
> Unfortunately i don't have the time to implement all of them.

From what I can see the driver does not implement the 5V wall
charger handling. It simply assumes, that the 5V wall charger
is there.

While you can skip implementation of battery connection check
you should make sure, that AC is actually connected.

> The Freescale downstream kernel (based on 2.6.35) [1] has a driver to
> handle all MXS power sources. But i think a clean implementation from
> the scratch would be necessary for mainline.
>
> > This would leave an almost empty driver, which just sets a single
> > register at probe time and probes more drivers. That would probably
> > be more fitting for the MFD subsystem.
> 
> I don't have any problems with fitting the driver to MFD.

You can keep in in drivers/power. From your code I assumed there
would simply be a fixed AC connection and nothing else.

> > -- Sebastian
> 
> [1] -
> http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/drivers/power/mxs/linux.c?h=imx_2.6.35_maintain

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH V2 3/6] power: mxs_power: add driver for mxs power subsystem

[Stefan Wahren]

Hi Sebastian,

Am 27.05.2015 um 15:51 schrieb Sebastian Reichel:
> Hi Stefan,
>
> On Wed, May 27, 2015 at 09:12:48AM +0200, Stefan Wahren wrote:
>> Am 23.05.2015 um 19:41 schrieb Sebastian Reichel:
>>> On Wed, Apr 29, 2015 at 10:32:24PM +0000, Stefan Wahren wrote:
>>>> This patch adds a minimal driver for the Freescale i.MX23, i.MX28
>>>> power subsystem. It's required to trigger the probing of the underlying
>>>> drivers like on-chip regulators.
>>> I'm not sure what you need the power supply class for. I see only
>>> one property (ac connected), which is constant. Maybe just drop
>>> the power supply stuff?
>> those SoCs could be powered by 3 different sources:
>> - 5V wall
>> - USB
>> - battery
>>
>> I choose the first one as a base, because it's the simplest one.
>> Unfortunately i don't have the time to implement all of them.
> From what I can see the driver does not implement the 5V wall
> charger handling. It simply assumes, that the 5V wall charger
> is there.

the driver consists of multiple files in the mxs sub directory.
I linked to the main file to reduce search effort. The battery
handling incl. charging should be implemented in ddi_power_battery.c

>From my understanding it's not possible to determine where the 5V
exactly come from (either 5V wall plug or USB). The driver simply uses
the USB PHY state.

>
> While you can skip implementation of battery connection check
> you should make sure, that AC is actually connected.

Do you refer to the right state of property POWER_SUPPLY_PROP_ONLINE or
driver probing?

Stefan


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 3/6] power: mxs_power: add driver for mxs power subsystem

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 858 bytes --]

Hi,

On Wed, May 27, 2015 at 06:34:01PM +0200, Stefan Wahren wrote:
> the driver consists of multiple files in the mxs sub directory.
> I linked to the main file to reduce search effort. The battery
> handling incl. charging should be implemented in ddi_power_battery.c
>
> From my understanding it's not possible to determine where the 5V
> exactly come from (either 5V wall plug or USB). The driver simply uses
> the USB PHY state.

Yes, but your driver does not even check, that there is 5V. It even
reports AC online if neither USB nor wall charger is connected.

> > While you can skip implementation of battery connection check
> > you should make sure, that AC is actually connected.
> 
> Do you refer to the right state of property
> POWER_SUPPLY_PROP_ONLINE or driver probing?

I'm referring to the right state.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH V2 4/6] DT: add binding for mxs on-chip regulators

[Stefan Wahren]

This patch adds the device tree bindings for the Freescale MXS on-chip
regulators.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../bindings/regulator/mxs-regulator.txt           |   65 ++++++++++++++++++++
 1 file changed, 65 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/mxs-regulator.txt

diff --git a/Documentation/devicetree/bindings/regulator/mxs-regulator.txt b/Documentation/devicetree/bindings/regulator/mxs-regulator.txt
new file mode 100644
index 0000000..e7092fc
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/mxs-regulator.txt
@@ -0,0 +1,65 @@
+Regulators on Freescale i.MX23/i.MX28
+
+On-chip linear regulator daisy chain:
+
++------------+     VDD5V  +---------+  VDDIO
+| DC-DC      |   +--------> LINREG  +--------->+
+|            |            | VDDIO   |          |
+|            |            +---------+          |
+| DCDC_VDDIO +->-+-----------------------------+
+|            |   |
+|            |   | VDDIO  +---------+  VDDA
+|            |   +--------> LINREG  +--------->+
+|            |            | VDDA    |          |
+|            |            +---------+          |
+| DCDC_VDDA  +->-+-----------------------------+
+|            |   |
+|            |   | VDDA   +---------+  VDD1P5
+|            |   +--------> LINREG  +--------->
+|            |   |        | VDD1P5  |
+|            |   |        +---------+
+|            |   | VDDA   +---------+  VDDD
+|            |   +--------> LINREG  +--------->+
+|            |            | VDDD    |          |
+|            |            +---------+          |
+| DCDC_VDDD  +->-------------------------------+
++------------+                                 |
+                                               v
+
+Required properties:
+- compatible (imx23 and imx28 supported):
+  - "fsl,<soc>-dcdc" for DC-DC converter
+  - "fsl,<soc>-vddd" for VDDD linear regulator
+  - "fsl,<soc>-vdda" for VDDA linear regulator
+  - "fsl,<soc>-vddio" for VDDIO linear regulator
+
+Optional properties:
+- switching-frequency: switching frequency for the DC-DC converter in Hz.
+  Possible values are <1200000>, <1250000> or <1500000> (default).
+
+Any regulator property defined as part of the core regulator
+binding, defined in regulator.txt, can also be used.
+
+Example for i.MX28:
+
+	power: power@80044000 {
+		compatible = "fsl,imx28-power", "syscon";
+		reg = <0x80044000 0x2000>;
+		interrupts = <6>;
+
+		dcdc: regulator@1 {
+			compatible = "fsl,imx28-dcdc";
+			regulator-name = "dcdc";
+			regulator-boot-on;
+			regulator-always-on;
+			switching-frequency = <1250000>;
+		};
+
+		reg_vddd: regulator@2 {
+			compatible = "fsl,imx28-vddd";
+			regulator-name = "vddd";
+			regulator-min-microvolt = <1350000>;
+			regulator-max-microvolt = <1550000>;
+		};
+	};
+
-- 
1.7.9.5

Re: [PATCH V2 4/6] DT: add binding for mxs on-chip regulators

[Jürgen Borleis]

Hi Stefan,

for the i.MX23 the regulator daisy chain looks a little bit different:

+------------+     VDD5V  +---------+  VDDIO
| DC-DC      |   +--------> LINREG  +--------->+
|            |            | VDDIO   |          |
|            |            +---------+          |
| DCDC_VDDIO +->-+-----------------------------+
|            |   |
|            |   | VDDIO  +---------+  VDDMEM
|            |   +--------> LINREG  +--------->
|            |   |        | VDDMEM  |
|            |   |        +---------+
|            |   | VDDIO  +---------+  VDDA
|            |   +--------> LINREG  +--------->+
|            |            | VDDA    |          |
|            |            +---------+          |
| DCDC_VDDA  +->-+-----------------------------+
|            |   | VDDA   +---------+  VDDD
|            |   +--------> LINREG  +--------->+
|            |            | VDDD    |          |
|            |            +---------+          |
| DCDC_VDDD  +->-------------------------------+
+------------+                                 |
                                               v

Regards,
Juergen

Re: [PATCH V2 4/6] DT: add binding for mxs on-chip regulators

[Stefan Wahren]

Hello Juergen,

> Jürgen Borleis <jbe@pengutronix.de> hat am 4. Mai 2015 um 15:51 geschrieben:
>
>
> Hi Stefan,
>
> for the i.MX23 the regulator daisy chain looks a little bit different:
>
> +------------+     VDD5V  +---------+  VDDIO
> | DC-DC      |   +--------> LINREG  +--------->+
> |            |            | VDDIO   |          |
> |            |            +---------+          |
> | DCDC_VDDIO +->-+-----------------------------+
> |            |   |
> |            |   | VDDIO  +---------+ VDDMEM
> |            |   +--------> LINREG  +--------->
> |            |   |        | VDDMEM  |
> |            |   |        +---------+
> |            |   | VDDIO  +---------+  VDDA
> |            |   +--------> LINREG  +--------->+
> |            |            | VDDA    |          |
> |            |            +---------+          |
> | DCDC_VDDA  +->-+-----------------------------+
> |            |   | VDDA   +---------+  VDDD
> |            |   +--------> LINREG  +--------->+
> |            |            | VDDD    |          |
> |            |            +---------+          |
> | DCDC_VDDD  +->-------------------------------+
> +------------+                                 |
>                                                v

damn! I didn't noticed that VDDMEM is powered by VDDIO and VDD1P5 is powered by
VDDA. Thank you very much.

From my understanding VDDMEM and VDD1P5 describe the same regulator function
(provides power to external memory such as SDRAM and mDDR which doesn't fit to
VDDD, VDDIO or VDDA). But the usage of
the name is not consistent across the documentation:
                   
                  | i.MX23 | i.MX28 
------------------------------------
Application note  | VDDMEM | VDD1P5
Reference manual  | VDDMEM | VDDMEM
Data sheet        | -      | VDD1P5

I would suggest to use VDDMEM, because the registers are always called HW_VDDMEM
(for both SoCs).

I think we have 3 options here:

a) add the regulator daisy chain for the i.MX23 too
b) remove VDD1P5 from the i.MX28 diagram so it would fit for both again
c) remove the diagram complete

I would prefer option b. What's your opinion?

>
> Regards,
> Juergen

Best regards
Stefan

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 4/6] DT: add binding for mxs on-chip regulators

[Jürgen Borleis]

Hi Stefan,

On Monday 04 May 2015 22:20:50 Stefan Wahren wrote:
> [...]
> >
> > +------------+     VDD5V  +---------+  VDDIO
> > | DC-DC      |   +--------> LINREG  +--------->+
> > |            |            | VDDIO   |          |
> > |            |            +---------+          |
> > |            |                                 |
> > | DCDC_VDDIO +->-+-----------------------------+
> > |            |   |
> > |            |   | VDDIO  +---------+ VDDMEM
> > |            |   +--------> LINREG  +--------->
> > |            |   |        | VDDMEM  |
> > |            |   |        +---------+
> > |            |   | VDDIO  +---------+  VDDA
> > |            |   +--------> LINREG  +--------->+
> > |            |            | VDDA    |          |
> > |            |            +---------+          |
> > | DCDC_VDDA  +->-+-----------------------------+
> > |            |   |
> > |            |   | VDDA   +---------+  VDDD
> > |            |   +--------> LINREG  +--------->+
> > |            |            | VDDD    |          |
> > |            |            +---------+          |
> > | DCDC_VDDD  +->-------------------------------+
> > +------------+                                 |
> >                                                v
>
> damn! I didn't noticed that VDDMEM is powered by VDDIO and VDD1P5 is
> powered by VDDA. Thank you very much.
>
> From my understanding VDDMEM and VDD1P5 describe the same regulator
> function (provides power to external memory such as SDRAM and mDDR which
> doesn't fit to VDDD, VDDIO or VDDA). But the usage of
> the name is not consistent across the documentation:
>                   | i.MX23 | i.MX28
> ------------------------------------
> Application note  | VDDMEM | VDD1P5
> Reference manual  | VDDMEM | VDDMEM
> Data sheet        | -      | VDD1P5
>
> I would suggest to use VDDMEM, because the registers are always called
> HW_VDDMEM (for both SoCs).

Ack.

> I think we have 3 options here:
>
> a) add the regulator daisy chain for the i.MX23 too
> b) remove VDD1P5 from the i.MX28 diagram so it would fit for both again
> c) remove the diagram complete
>
> I would prefer option b. What's your opinion?

I prefer a) because once the details are known, we should keep them.

Regards,
Juergen

Re: [PATCH V2 4/6] DT: add binding for mxs on-chip regulators

[Stefan Wahren]

Hi Juergen,

> Jürgen Borleis <jbe-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> hat am 5. Mai 2015 um 10:21 geschrieben:
>
>
> Hi Stefan,
>
> On Monday 04 May 2015 22:20:50 Stefan Wahren wrote:
> > [...]
> > 
> > I think we have 3 options here:
> >
> > a) add the regulator daisy chain for the i.MX23 too
> > b) remove VDD1P5 from the i.MX28 diagram so it would fit for both again
> > c) remove the diagram complete
> >
> > I would prefer option b. What's your opinion?
>
> I prefer a) because once the details are known, we should keep them.

beside the point this could be to much diagrams for a binding documentation, i
don't have any objections about option a).

>
> Regards,
> Juergen

Regards
Stefan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 5/6] regulator: add mxs on-chip regulator driver

[Stefan Wahren]

This patch adds driver support for Freescale i.MX23, i.MX28
on-chip regulators. The driver supports the following components:
dcdc, vddd, vdda and vddio.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 drivers/regulator/Kconfig         |    8 +
 drivers/regulator/Makefile        |    1 +
 drivers/regulator/mxs-regulator.c |  540 +++++++++++++++++++++++++++++++++++++
 3 files changed, 549 insertions(+)
 create mode 100644 drivers/regulator/mxs-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index a6f116a..7b525f5 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -450,6 +450,14 @@ config REGULATOR_MT6397
 	  This driver supports the control of different power rails of device
 	  through regulator interface.
 
+config REGULATOR_MXS
+	tristate "Freescale MXS on-chip regulators"
+	depends on (MXS_POWER || COMPILE_TEST)
+	help
+	  Say y here to support Freescale MXS on-chip regulators.
+	  It is recommended that this option be enabled on i.MX23,
+	  i.MX28 platform.
+
 config REGULATOR_PALMAS
 	tristate "TI Palmas PMIC Regulators"
 	depends on MFD_PALMAS
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 2c4da15..a3ebf23 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -60,6 +60,7 @@ obj-$(CONFIG_REGULATOR_MC13783) += mc13783-regulator.o
 obj-$(CONFIG_REGULATOR_MC13892) += mc13892-regulator.o
 obj-$(CONFIG_REGULATOR_MC13XXX_CORE) +=  mc13xxx-regulator-core.o
 obj-$(CONFIG_REGULATOR_MT6397)	+= mt6397-regulator.o
+obj-$(CONFIG_REGULATOR_MXS) += mxs-regulator.o
 obj-$(CONFIG_REGULATOR_QCOM_RPM) += qcom_rpm-regulator.o
 obj-$(CONFIG_REGULATOR_PALMAS) += palmas-regulator.o
 obj-$(CONFIG_REGULATOR_PFUZE100) += pfuze100-regulator.o
diff --git a/drivers/regulator/mxs-regulator.c b/drivers/regulator/mxs-regulator.c
new file mode 100644
index 0000000..e53707b
--- /dev/null
+++ b/drivers/regulator/mxs-regulator.c
@@ -0,0 +1,540 @@
+/*
+ * Freescale MXS on-chip regulators
+ *
+ * Embedded Alley Solutions, Inc <source@embeddedalley.com>
+ *
+ * Copyright (C) 2014 Stefan Wahren
+ * Copyright (C) 2010 Freescale Semiconductor, Inc.
+ * Copyright (C) 2008 Embedded Alley Solutions, Inc All Rights Reserved.
+ *
+ * Inspired by imx-bootlets
+ */
+
+/*
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/slab.h>
+
+/* Powered by linear regulator.  DCDC output is gated off and
+   the linreg output is equal to the target. */
+#define HW_POWER_LINREG_DCDC_OFF		1
+
+/* Powered by linear regulator.  DCDC output is not gated off
+   and is ready for the automatic hardware transistion after a 5V
+   event.  The converters are not enabled when 5V is present. LinReg output
+   is 25mV below target. */
+#define HW_POWER_LINREG_DCDC_READY		2
+
+/* Powered by DCDC converter and the LinReg is on. LinReg output
+   is 25mV below target. */
+#define HW_POWER_DCDC_LINREG_ON			3
+
+/* Powered by DCDC converter and the LinReg is off. LinReg output
+   is 25mV below target. */
+#define HW_POWER_DCDC_LINREG_OFF		4
+
+/* Powered by DCDC converter and the LinReg is ready for the
+   automatic hardware transfer.  The LinReg output is not enabled and
+   depends on the 5V presence to enable the LinRegs.  LinReg offset is 25mV
+   below target. */
+#define HW_POWER_DCDC_LINREG_READY		5
+
+/* Powered by an external source when 5V is present.  This does not
+   necessarily mean the external source is powered by 5V,but the chip needs
+   to be aware that 5V is present. */
+#define HW_POWER_EXTERNAL_SOURCE_5V		6
+
+/* Powered by an external source when 5V is not present.This doesn't
+   necessarily mean the external source is powered by the battery, but the
+   chip needs to be aware that the battery is present */
+#define HW_POWER_EXTERNAL_SOURCE_BATTERY	7
+
+/* Unknown configuration.  This is an error. */
+#define HW_POWER_UNKNOWN_SOURCE			8
+
+/* TODO: Move power register offsets into header file */
+#define HW_POWER_5VCTRL		0x00000010
+#define HW_POWER_VDDDCTRL	0x00000040
+#define HW_POWER_VDDACTRL	0x00000050
+#define HW_POWER_VDDIOCTRL	0x00000060
+#define HW_POWER_MISC		0x00000090
+#define HW_POWER_STS		0x000000c0
+
+#define BM_POWER_STS_VBUSVALID0_STATUS	BIT(15)
+#define BM_POWER_STS_DC_OK		BIT(9)
+
+#define BM_POWER_5VCTRL_ILIMIT_EQ_ZERO	BIT(2)
+#define BM_POWER_5VCTRL_ENABLE_DCDC	BIT(0)
+
+#define BM_POWER_LINREG_OFFSET_DCDC_MODE	BIT(1)
+
+#define SHIFT_FREQSEL			4
+
+#define BM_POWER_MISC_FREQSEL		(7 << SHIFT_FREQSEL)
+
+/* Recommended DC-DC clock source values */
+#define HW_POWER_MISC_FREQSEL_20000_KHZ	1
+#define HW_POWER_MISC_FREQSEL_24000_KHZ	2
+#define HW_POWER_MISC_FREQSEL_19200_KHZ	3
+
+#define HW_POWER_MISC_SEL_PLLCLK	BIT(0)
+
+/* Regulator IDs */
+#define MXS_DCDC	1
+#define MXS_VDDIO	2
+#define MXS_VDDA	3
+#define MXS_VDDD	4
+
+struct mxs_reg_info {
+	/* regulator descriptor */
+	struct regulator_desc desc;
+
+	/* regulator control register */
+	int ctrl_reg;
+
+	/* disable DC-DC output */
+	unsigned int disable_fet_mask;
+
+	/* steps between linreg output and DC-DC target */
+	unsigned int linreg_offset_mask;
+	u8 linreg_offset_shift;
+
+	/* function which determine power source */
+	u8 (*get_power_source)(struct regulator_dev *);
+};
+
+static inline u8 get_linreg_offset(struct mxs_reg_info *ldo, u32 regs)
+{
+	return (regs & ldo->linreg_offset_mask) >> ldo->linreg_offset_shift;
+}
+
+static u8 get_vddio_power_source(struct regulator_dev *reg)
+{
+	struct mxs_reg_info *ldo = rdev_get_drvdata(reg);
+	u32 v5ctrl, status, base;
+	u8 offset;
+
+	if (regmap_read(reg->regmap, HW_POWER_5VCTRL, &v5ctrl))
+		return HW_POWER_UNKNOWN_SOURCE;
+
+	if (regmap_read(reg->regmap, HW_POWER_STS, &status))
+		return HW_POWER_UNKNOWN_SOURCE;
+
+	if (regmap_read(reg->regmap, ldo->ctrl_reg, &base))
+		return HW_POWER_UNKNOWN_SOURCE;
+
+	offset = get_linreg_offset(ldo, base);
+
+	/* If VBUS valid then 5 V power supply present */
+	if (status & BM_POWER_STS_VBUSVALID0_STATUS) {
+		/* Powered by Linreg, DC-DC is off */
+		if ((base & ldo->disable_fet_mask) &&
+		    !(offset & BM_POWER_LINREG_OFFSET_DCDC_MODE)) {
+			return HW_POWER_LINREG_DCDC_OFF;
+		}
+
+		if (v5ctrl & BM_POWER_5VCTRL_ENABLE_DCDC) {
+			/* Powered by DC-DC, Linreg is on */
+			if (offset & BM_POWER_LINREG_OFFSET_DCDC_MODE)
+				return HW_POWER_DCDC_LINREG_ON;
+		} else {
+			/* Powered by Linreg, DC-DC is off */
+			if (!(offset & BM_POWER_LINREG_OFFSET_DCDC_MODE))
+				return HW_POWER_LINREG_DCDC_OFF;
+		}
+	} else {
+		/* Powered by DC-DC, Linreg is on */
+		if (offset & BM_POWER_LINREG_OFFSET_DCDC_MODE)
+			return HW_POWER_DCDC_LINREG_ON;
+	}
+
+	return HW_POWER_UNKNOWN_SOURCE;
+}
+
+static u8 get_vdda_vddd_power_source(struct regulator_dev *reg)
+{
+	struct mxs_reg_info *ldo = rdev_get_drvdata(reg);
+	struct regulator_desc *desc = &ldo->desc;
+	u32 v5ctrl, status, base;
+	u8 offset;
+
+	if (regmap_read(reg->regmap, HW_POWER_5VCTRL, &v5ctrl))
+		return HW_POWER_UNKNOWN_SOURCE;
+
+	if (regmap_read(reg->regmap, HW_POWER_STS, &status))
+		return HW_POWER_UNKNOWN_SOURCE;
+
+	if (regmap_read(reg->regmap, ldo->ctrl_reg, &base))
+		return HW_POWER_UNKNOWN_SOURCE;
+
+	offset = get_linreg_offset(ldo, base);
+
+	/* DC-DC output is disabled */
+	if (base & ldo->disable_fet_mask) {
+		/* Powered by 5 V supply */
+		if (status & BM_POWER_STS_VBUSVALID0_STATUS)
+			return HW_POWER_EXTERNAL_SOURCE_5V;
+
+		/* Powered by Linreg, DC-DC is off */
+		if (!(offset & BM_POWER_LINREG_OFFSET_DCDC_MODE))
+			return HW_POWER_LINREG_DCDC_OFF;
+	}
+
+	/* If VBUS valid then 5 V power supply present */
+	if (status & BM_POWER_STS_VBUSVALID0_STATUS) {
+		/* Powered by DC-DC, Linreg is on */
+		if (v5ctrl & BM_POWER_5VCTRL_ENABLE_DCDC)
+			return HW_POWER_DCDC_LINREG_ON;
+
+		/* Powered by Linreg, DC-DC is off */
+		return HW_POWER_LINREG_DCDC_OFF;
+	}
+
+	/* DC-DC is on */
+	if (offset & BM_POWER_LINREG_OFFSET_DCDC_MODE) {
+		/* Powered by DC-DC, Linreg is on */
+		if (base & desc->enable_mask)
+			return HW_POWER_DCDC_LINREG_ON;
+
+		/* Powered by DC-DC, Linreg is off */
+		return HW_POWER_DCDC_LINREG_OFF;
+	}
+
+	return HW_POWER_UNKNOWN_SOURCE;
+}
+
+static int mxs_set_dcdc_freq(struct regulator_dev *reg, u32 hz)
+{
+	struct mxs_reg_info *dcdc = rdev_get_drvdata(reg);
+	u32 val;
+	int ret;
+
+	if (dcdc->desc.id != MXS_DCDC) {
+		dev_warn(&reg->dev, "Setting switching freq is not supported\n");
+		return -EINVAL;
+	}
+
+	ret = regmap_read(reg->regmap, HW_POWER_MISC, &val);
+	if (ret)
+		return ret;
+
+	val &= ~BM_POWER_MISC_FREQSEL;
+	val &= ~HW_POWER_MISC_SEL_PLLCLK;
+
+	/*
+	 * Select the PLL/PFD based frequency that the DC-DC converter uses.
+	 * The actual switching frequency driving the power inductor is
+	 * DCDC_CLK/16. Accept only values recommend by Freescale.
+	 */
+	switch (hz) {
+	case 1200000:
+		val |= HW_POWER_MISC_FREQSEL_19200_KHZ << SHIFT_FREQSEL;
+		break;
+	case 1250000:
+		val |= HW_POWER_MISC_FREQSEL_20000_KHZ << SHIFT_FREQSEL;
+		break;
+	case 1500000:
+		val |= HW_POWER_MISC_FREQSEL_24000_KHZ << SHIFT_FREQSEL;
+		break;
+	default:
+		dev_warn(&reg->dev, "Switching freq: %u Hz not supported\n",
+			 hz);
+		return -EINVAL;
+	}
+
+	/* First program FREQSEL */
+	ret = regmap_write(reg->regmap, HW_POWER_MISC, val);
+	if (ret)
+		return ret;
+
+	/* then set PLL as clock for DC-DC converter */
+	val |= HW_POWER_MISC_SEL_PLLCLK;
+
+	return regmap_write(reg->regmap, HW_POWER_MISC, val);
+}
+
+static int mxs_ldo_set_voltage_sel(struct regulator_dev *reg, unsigned sel)
+{
+	struct mxs_reg_info *ldo = rdev_get_drvdata(reg);
+	struct regulator_desc *desc = &ldo->desc;
+	u32 status = 0;
+	int timeout;
+	int ret;
+
+	ret = regmap_update_bits(reg->regmap, desc->vsel_reg, desc->vsel_mask,
+				 sel);
+	if (ret)
+		return ret;
+
+	if (ldo->get_power_source) {
+		switch (ldo->get_power_source(reg)) {
+		case HW_POWER_LINREG_DCDC_OFF:
+		case HW_POWER_LINREG_DCDC_READY:
+		case HW_POWER_EXTERNAL_SOURCE_5V:
+			/*
+			 * Since the DC-DC converter is off we can't
+			 * trigger on DC_OK. So wait at least 1 ms
+			 * for stabilization.
+			 */
+			usleep_range(1000, 2000);
+			return 0;
+		}
+	}
+
+	/* Make sure DC_OK has changed */
+	usleep_range(15, 20);
+
+	for (timeout = 0; timeout < 20; timeout++) {
+		ret = regmap_read(reg->regmap, HW_POWER_STS, &status);
+
+		if (ret)
+			break;
+
+		/* DC-DC converter control loop has stabilized */
+		if (status & BM_POWER_STS_DC_OK)
+			return 0;
+
+		udelay(1);
+	}
+
+	if (!ret)
+		dev_warn_ratelimited(&reg->dev, "%s: timeout status=0x%08x\n",
+				     __func__, status);
+
+	msleep(20);
+
+	return -ETIMEDOUT;
+}
+
+static int mxs_ldo_is_enabled(struct regulator_dev *reg)
+{
+	struct mxs_reg_info *ldo = rdev_get_drvdata(reg);
+
+	if (ldo->get_power_source) {
+		switch (ldo->get_power_source(reg)) {
+		case HW_POWER_LINREG_DCDC_OFF:
+		case HW_POWER_LINREG_DCDC_READY:
+		case HW_POWER_DCDC_LINREG_ON:
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
+static struct regulator_ops mxs_dcdc_ops = {
+	.is_enabled		= regulator_is_enabled_regmap,
+};
+
+static struct regulator_ops mxs_ldo_ops = {
+	.list_voltage		= regulator_list_voltage_linear,
+	.map_voltage		= regulator_map_voltage_linear,
+	.set_voltage_sel	= mxs_ldo_set_voltage_sel,
+	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
+	.is_enabled		= mxs_ldo_is_enabled,
+};
+
+static const struct mxs_reg_info mxs_info_dcdc = {
+	.desc = {
+		.name = "dcdc",
+		.id = MXS_DCDC,
+		.type = REGULATOR_VOLTAGE,
+		.owner = THIS_MODULE,
+		.ops = &mxs_dcdc_ops,
+		.enable_reg = HW_POWER_STS,
+		.enable_mask = (1 << 0),
+	},
+};
+
+static const struct mxs_reg_info imx23_info_vddio = {
+	.desc = {
+		.name = "vddio",
+		.id = MXS_VDDIO,
+		.type = REGULATOR_VOLTAGE,
+		.owner = THIS_MODULE,
+		.n_voltages = 0x20,
+		.uV_step = 25000,
+		.linear_min_sel = 0,
+		.min_uV = 2800000,
+		.vsel_reg = HW_POWER_VDDIOCTRL,
+		.vsel_mask = 0x1f,
+		.ops = &mxs_ldo_ops,
+	},
+	.ctrl_reg = HW_POWER_VDDIOCTRL,
+	.disable_fet_mask = 1 << 16,
+	.linreg_offset_mask = 3 << 12,
+	.linreg_offset_shift = 12,
+	.get_power_source = get_vddio_power_source,
+};
+
+static const struct mxs_reg_info imx28_info_vddio = {
+	.desc = {
+		.name = "vddio",
+		.id = MXS_VDDIO,
+		.type = REGULATOR_VOLTAGE,
+		.owner = THIS_MODULE,
+		.n_voltages = 0x11,
+		.uV_step = 50000,
+		.linear_min_sel = 0,
+		.min_uV = 2800000,
+		.vsel_reg = HW_POWER_VDDIOCTRL,
+		.vsel_mask = 0x1f,
+		.ops = &mxs_ldo_ops,
+	},
+	.ctrl_reg = HW_POWER_VDDIOCTRL,
+	.disable_fet_mask = 1 << 16,
+	.linreg_offset_mask = 3 << 12,
+	.linreg_offset_shift = 12,
+	.get_power_source = get_vddio_power_source,
+};
+
+static const struct mxs_reg_info mxs_info_vdda = {
+	.desc = {
+		.name = "vdda",
+		.id = MXS_VDDA,
+		.type = REGULATOR_VOLTAGE,
+		.owner = THIS_MODULE,
+		.n_voltages = 0x20,
+		.uV_step = 25000,
+		.linear_min_sel = 0,
+		.min_uV = 1500000,
+		.vsel_reg = HW_POWER_VDDACTRL,
+		.vsel_mask = 0x1f,
+		.ops = &mxs_ldo_ops,
+		.enable_mask = (1 << 17),
+	},
+	.ctrl_reg = HW_POWER_VDDACTRL,
+	.disable_fet_mask = 1 << 16,
+	.linreg_offset_mask = 3 << 12,
+	.linreg_offset_shift = 12,
+	.get_power_source = get_vdda_vddd_power_source,
+};
+
+static const struct mxs_reg_info mxs_info_vddd = {
+	.desc = {
+		.name = "vddd",
+		.id = MXS_VDDD,
+		.type = REGULATOR_VOLTAGE,
+		.owner = THIS_MODULE,
+		.n_voltages = 0x20,
+		.uV_step = 25000,
+		.linear_min_sel = 0,
+		.min_uV = 800000,
+		.vsel_reg = HW_POWER_VDDDCTRL,
+		.vsel_mask = 0x1f,
+		.ops = &mxs_ldo_ops,
+		.enable_mask = (1 << 21),
+	},
+	.ctrl_reg = HW_POWER_VDDDCTRL,
+	.disable_fet_mask = 1 << 20,
+	.linreg_offset_mask = 3 << 16,
+	.linreg_offset_shift = 16,
+	.get_power_source = get_vdda_vddd_power_source,
+};
+
+static const struct of_device_id of_mxs_regulator_match[] = {
+	{ .compatible = "fsl,imx23-dcdc",  .data = &mxs_info_dcdc },
+	{ .compatible = "fsl,imx28-dcdc",  .data = &mxs_info_dcdc },
+	{ .compatible = "fsl,imx23-vddio", .data = &imx23_info_vddio },
+	{ .compatible = "fsl,imx23-vdda",  .data = &mxs_info_vdda },
+	{ .compatible = "fsl,imx23-vddd",  .data = &mxs_info_vddd },
+	{ .compatible = "fsl,imx28-vddio", .data = &imx28_info_vddio },
+	{ .compatible = "fsl,imx28-vdda",  .data = &mxs_info_vdda },
+	{ .compatible = "fsl,imx28-vddd",  .data = &mxs_info_vddd },
+	{ /* end */ }
+};
+MODULE_DEVICE_TABLE(of, of_mxs_regulator_match);
+
+static int mxs_regulator_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct of_device_id *match;
+	struct device_node *parent_np;
+	struct regulator_dev *rdev = NULL;
+	struct mxs_reg_info *info;
+	struct regulator_init_data *initdata;
+	struct regulator_config config = { };
+	u32 switch_freq;
+
+	match = of_match_device(of_mxs_regulator_match, dev);
+	if (!match) {
+		/* We do not expect this to happen */
+		dev_err(dev, "%s: Unable to match device\n", __func__);
+		return -ENODEV;
+	}
+
+	info = devm_kmemdup(dev, match->data, sizeof(struct mxs_reg_info),
+			    GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	initdata = of_get_regulator_init_data(dev, dev->of_node, &info->desc);
+	if (!initdata) {
+		dev_err(dev, "missing regulator init data\n");
+		return -EINVAL;
+	}
+
+	parent_np = of_get_parent(dev->of_node);
+	if (!parent_np)
+		return -ENODEV;
+	config.regmap = syscon_node_to_regmap(parent_np);
+	of_node_put(parent_np);
+	if (IS_ERR(config.regmap))
+		return PTR_ERR(config.regmap);
+
+	config.dev = dev;
+	config.init_data = initdata;
+	config.driver_data = info;
+	config.of_node = dev->of_node;
+
+	rdev = devm_regulator_register(dev, &info->desc, &config);
+	if (IS_ERR(rdev)) {
+		int ret = PTR_ERR(rdev);
+
+		dev_err(dev, "%s: failed to register regulator(%d)\n",
+			__func__, ret);
+		return ret;
+	}
+
+	if (!of_property_read_u32(dev->of_node, "switching-frequency",
+				  &switch_freq))
+		mxs_set_dcdc_freq(rdev, switch_freq);
+
+	platform_set_drvdata(pdev, rdev);
+
+	return 0;
+}
+
+static struct platform_driver mxs_regulator_driver = {
+	.driver = {
+		.name	= "mxs_regulator",
+		.of_match_table = of_mxs_regulator_match,
+	},
+	.probe	= mxs_regulator_probe,
+};
+
+module_platform_driver(mxs_regulator_driver);
+
+MODULE_AUTHOR("Stefan Wahren <stefan.wahren@i2se.com>");
+MODULE_DESCRIPTION("Freescale MXS regulators");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:mxs_regulator");
-- 
1.7.9.5

Re: [PATCH V2 5/6] regulator: add mxs on-chip regulator driver

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1337 bytes --]

On Wed, Apr 29, 2015 at 10:32:26PM +0000, Stefan Wahren wrote:

> +static struct regulator_ops mxs_dcdc_ops = {
> +	.is_enabled		= regulator_is_enabled_regmap,
> +};

Why do we have an is_enabled() operation but no enable or disable
operation?  I'm also not 100% clear what code the DCDCs and LDOs are
sharing here...

> +	initdata = of_get_regulator_init_data(dev, dev->of_node, &info->desc);
> +	if (!initdata) {
> +		dev_err(dev, "missing regulator init data\n");
> +		return -EINVAL;
> +	}

This is not an error, init data is totally optional.

> +	rdev = devm_regulator_register(dev, &info->desc, &config);
> +	if (IS_ERR(rdev)) {
> +		int ret = PTR_ERR(rdev);
> +
> +		dev_err(dev, "%s: failed to register regulator(%d)\n",
> +			__func__, ret);
> +		return ret;
> +	}
> +
> +	if (!of_property_read_u32(dev->of_node, "switching-frequency",
> +				  &switch_freq))
> +		mxs_set_dcdc_freq(rdev, switch_freq);

why are we registering the regulator before we've finished parsing the
DT - I'd expect us to do this first rather than have things potentially
start using the regulator with the confguration not completed.

> +	platform_set_drvdata(pdev, rdev);

This looks unsafe - either we don't need to set this at all or one of
the ops could be invoked when the regulator is registered and try to use
the driver data before it is set.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 5/6] regulator: add mxs on-chip regulator driver

[Stefan Wahren]

Hi Mark,

> Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> hat am 4. Mai 2015 um 14:36 geschrieben:
>
>
> On Wed, Apr 29, 2015 at 10:32:26PM +0000, Stefan Wahren wrote:
>
> > +static struct regulator_ops mxs_dcdc_ops = {
> > + .is_enabled = regulator_is_enabled_regmap,
> > +};
>
> Why do we have an is_enabled() operation but no enable or disable
> operation? 

i'm sorry, i forgot the following point in the changelog:

- move dcdc handling from mxs_power to mxs-regulator

I hope the change is okay. I implemented only is_enabled() to have at least 1
operation.
But it's not really necessary.

> I'm also not 100% clear what code the DCDCs and LDOs are
> sharing here...

Actually only probing and the regulator info structure.

Best regards
Stefan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 5/6] regulator: add mxs on-chip regulator driver

[Stefan Wahren]

Hi Mark,

> Stefan Wahren <stefan.wahren@i2se.com> hat am 4. Mai 2015 um 22:34
> geschrieben:
>
>
> Hi Mark,
>
> > Mark Brown <broonie@kernel.org> hat am 4. Mai 2015 um 14:36 geschrieben:
> >
> >
> > On Wed, Apr 29, 2015 at 10:32:26PM +0000, Stefan Wahren wrote:
> >
> > > +static struct regulator_ops mxs_dcdc_ops = {
> > > + .is_enabled = regulator_is_enabled_regmap,
> > > +};
> >
> > Why do we have an is_enabled() operation but no enable or disable
> > operation?
>
> i'm sorry, i forgot the following point in the changelog:
>
> - move dcdc handling from mxs_power to mxs-regulator
>
> I hope the change is okay. I implemented only is_enabled() to have at least 1
> operation.
> But it's not really necessary.

do we need enable / disable operation?

In the bootloader code enabling the DC-DC is critical. There is a known issue
which falsely triggers the brownout detection.

>
> > I'm also not 100% clear what code the DCDCs and LDOs are
> > sharing here...
>
> Actually only probing and the regulator info structure.

How can i improve that?

>
> Best regards
> Stefan

Re: [PATCH V2 5/6] regulator: add mxs on-chip regulator driver

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 860 bytes --]

On Thu, May 07, 2015 at 08:37:18PM +0200, Stefan Wahren wrote:
> > Stefan Wahren <stefan.wahren@i2se.com> hat am 4. Mai 2015 um 22:34

> > i'm sorry, i forgot the following point in the changelog:

> > - move dcdc handling from mxs_power to mxs-regulator

> > I hope the change is okay. I implemented only is_enabled() to have at least 1
> > operation.
> > But it's not really necessary.

> do we need enable / disable operation?

> In the bootloader code enabling the DC-DC is critical. There is a known issue
> which falsely triggers the brownout detection.

Why would the driver not provide these operations?

> > > I'm also not 100% clear what code the DCDCs and LDOs are
> > > sharing here...

> > Actually only probing and the regulator info structure.

> How can i improve that?

If the devices aren't sharing code then they should be separate drivers.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH V2 5/6] regulator: add mxs on-chip regulator driver

[Stefan Wahren]

> Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> hat am 7. Mai 2015 um 20:52 geschrieben:
>
>
> On Thu, May 07, 2015 at 08:37:18PM +0200, Stefan Wahren wrote:
> > > Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> hat am 4. Mai 2015 um 22:34
>
> > > i'm sorry, i forgot the following point in the changelog:
>
> > > - move dcdc handling from mxs_power to mxs-regulator
>
> > > I hope the change is okay. I implemented only is_enabled() to have at
> > > least 1
> > > operation.
> > > But it's not really necessary.
>
> > do we need enable / disable operation?
>
> > In the bootloader code enabling the DC-DC is critical. There is a known
> > issue
> > which falsely triggers the brownout detection.
>
> Why would the driver not provide these operations?

I didn't implement this operation because the suggested workaround for the
hardware issue isn't very nice 
(disable brownout detection during enabling DC-DC). I was afraid of the
implementation.

>
> > > > I'm also not 100% clear what code the DCDCs and LDOs are
> > > > sharing here...
>
> > > Actually only probing and the regulator info structure.
>
> > How can i improve that?
>
> If the devices aren't sharing code then they should be separate drivers.

Okay. 

mxs-regulator-ldo.c
mxs-regulator-dcdc.c

What do you think of?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 5/6] regulator: add mxs on-chip regulator driver

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 773 bytes --]

On Thu, May 07, 2015 at 09:25:19PM +0200, Stefan Wahren wrote:
> > Mark Brown <broonie@kernel.org> hat am 7. Mai 2015 um 20:52 geschrieben:

> > > In the bootloader code enabling the DC-DC is critical. There is a known
> > > issue
> > > which falsely triggers the brownout detection.

> > Why would the driver not provide these operations?

> I didn't implement this operation because the suggested workaround for the
> hardware issue isn't very nice 
> (disable brownout detection during enabling DC-DC). I was afraid of the
> implementation.

That doesn't sound too complicated?

> > If the devices aren't sharing code then they should be separate drivers.

> Okay. 

> mxs-regulator-ldo.c
> mxs-regulator-dcdc.c

> What do you think of?

Sure.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH V2 5/6] regulator: add mxs on-chip regulator driver

[Stefan Wahren]

> Mark Brown <broonie@kernel.org> hat am 7. Mai 2015 um 21:32 geschrieben:
>
>
> On Thu, May 07, 2015 at 09:25:19PM +0200, Stefan Wahren wrote:
> > > Mark Brown <broonie@kernel.org> hat am 7. Mai 2015 um 20:52 geschrieben:
>
> > > > In the bootloader code enabling the DC-DC is critical. There is a known
> > > > issue
> > > > which falsely triggers the brownout detection.
>
> > > Why would the driver not provide these operations?
>
> > I didn't implement this operation because the suggested workaround for the
> > hardware issue isn't very nice
> > (disable brownout detection during enabling DC-DC). I was afraid of the
> > implementation.
>
> That doesn't sound too complicated?
>

Okay, i think i managed the brownout handling.

Unfortunately i'm stuck at the next step. The DC-DC (3 way output) can supply 3
LDOs vddd, vdda, vddio. In case of a DC-DC state change the 3 regulators needs
to be reconfigured before.

I tought of REGULATOR_EVENT_PRE_DISABLE and REGULATOR_EVENT_PRE_ENABLE events,
but i'm not sure that's the right way to achieve a synchronization between DC-DC
and consuming LDOs.

Re: [PATCH V2 5/6] regulator: add mxs on-chip regulator driver

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1455 bytes --]

On Tue, May 26, 2015 at 02:16:55PM +0200, Stefan Wahren wrote:
> > Mark Brown <broonie@kernel.org> hat am 7. Mai 2015 um 21:32 geschrieben:

> > > > > In the bootloader code enabling the DC-DC is critical. There is a known
> > > > > issue
> > > > > which falsely triggers the brownout detection.

> > > > Why would the driver not provide these operations?

> > > I didn't implement this operation because the suggested workaround for the
> > > hardware issue isn't very nice
> > > (disable brownout detection during enabling DC-DC). I was afraid of the
> > > implementation.

> > That doesn't sound too complicated?

> Okay, i think i managed the brownout handling.
> 
> Unfortunately i'm stuck at the next step. The DC-DC (3 way output) can supply 3
> LDOs vddd, vdda, vddio. In case of a DC-DC state change the 3 regulators needs
> to be reconfigured before.

> I tought of REGULATOR_EVENT_PRE_DISABLE and REGULATOR_EVENT_PRE_ENABLE events,
> but i'm not sure that's the right way to achieve a synchronization between DC-DC
> and consuming LDOs.

OK, that's possibly sounding a bit complicated.  Is it just a case of
writing some value, doing a reconfiguration and rewriting the original
value or is it something more?  If it's much more complicated than that
then I think putting a comment in the driver explaining what the issue
is would be the right step, that would address the issue with the driver
looking strange.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH V2 5/6] regulator: add mxs on-chip regulator driver

[Stefan Wahren]

> Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> hat am 27. Mai 2015 um 19:33 geschrieben:
>
>
> On Tue, May 26, 2015 at 02:16:55PM +0200, Stefan Wahren wrote:
> > > Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> hat am 7. Mai 2015 um 21:32 geschrieben:
>
> > > > > > In the bootloader code enabling the DC-DC is critical. There is a
> > > > > > known
> > > > > > issue
> > > > > > which falsely triggers the brownout detection.
>
> > > > > Why would the driver not provide these operations?
>
> > > > I didn't implement this operation because the suggested workaround for
> > > > the
> > > > hardware issue isn't very nice
> > > > (disable brownout detection during enabling DC-DC). I was afraid of the
> > > > implementation.
>
> > > That doesn't sound too complicated?
>
> > Okay, i think i managed the brownout handling.
> >
> > Unfortunately i'm stuck at the next step. The DC-DC (3 way output) can
> > supply 3
> > LDOs vddd, vdda, vddio. In case of a DC-DC state change the 3 regulators
> > needs
> > to be reconfigured before.
>
> > I tought of REGULATOR_EVENT_PRE_DISABLE and REGULATOR_EVENT_PRE_ENABLE
> > events,
> > but i'm not sure that's the right way to achieve a synchronization between
> > DC-DC
> > and consuming LDOs.
>
> OK, that's possibly sounding a bit complicated. Is it just a case of
> writing some value, doing a reconfiguration and rewriting the original
> value or is it something more?

In case of a single driver instance instead of 4 ones (vddd, vddio, vdda,
dc-dc), things would be much easier.

Here are the steps for disabling DC-DC:

1. configure vddio to work without DC-DC
2. configure vdda to work without DC-DC
3. configure vddd to work without DC-DC
4. backup DC-DC settings
5. disable brownout detection
6. disable DC-DC
7. restore DC-DC settings

My problem is that step 1 to 3 must be done step by step through the other
regulator instances.

> If it's much more complicated than that
> then I think putting a comment in the driver explaining what the issue
> is would be the right step, that would address the issue with the driver
> looking strange.

Enabling or disabling DC-DC is only needed internally by the power subsystem
(e.g. in case of handoff AC to battery), because the DC-DC only supplys the
regulator daisy. From my understanding hardware components could use only the
regulator outputs. In the end a state change is necessary, but it shouldn't be a
public.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 5/6] regulator: add mxs on-chip regulator driver

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1357 bytes --]

On Wed, May 27, 2015 at 10:58:01PM +0200, Stefan Wahren wrote:

> In case of a single driver instance instead of 4 ones (vddd, vddio, vdda,
> dc-dc), things would be much easier.

> Here are the steps for disabling DC-DC:

> 1. configure vddio to work without DC-DC
> 2. configure vdda to work without DC-DC
> 3. configure vddd to work without DC-DC
> 4. backup DC-DC settings
> 5. disable brownout detection
> 6. disable DC-DC
> 7. restore DC-DC settings

> My problem is that step 1 to 3 must be done step by step through the other
> regulator instances.

For the regulator API steps 1-3 aren't going to be needed since
dependency management means that we won't try to disable a regulator
that we think is in use.

> > If it's much more complicated than that
> > then I think putting a comment in the driver explaining what the issue
> > is would be the right step, that would address the issue with the driver
> > looking strange.

> Enabling or disabling DC-DC is only needed internally by the power subsystem
> (e.g. in case of handoff AC to battery), because the DC-DC only supplys the
> regulator daisy. From my understanding hardware components could use only the
> regulator outputs. In the end a state change is necessary, but it shouldn't be a
> public.

Should this actually be shown as a regulator then and not as part of the
power controller?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[PATCH V2 6/6] ARM: dts: enable regulator support for i.MX23/i.MX28

[Stefan Wahren]

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 |   40 +++++++++++++++++++++++++++++++++++++++-
 arch/arm/boot/dts/imx28.dtsi |   40 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 78 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/imx23.dtsi b/arch/arm/boot/dts/imx23.dtsi
index bbcfb5a..efdd702 100644
--- a/arch/arm/boot/dts/imx23.dtsi
+++ b/arch/arm/boot/dts/imx23.dtsi
@@ -404,8 +404,46 @@
 			};
 
 			power@80044000 {
+				compatible = "fsl,imx23-power", "syscon";
 				reg = <0x80044000 0x2000>;
-				status = "disabled";
+				interrupts = <3>;
+
+				dcdc: regulator@1 {
+					compatible = "fsl,imx23-dcdc";
+					regulator-name = "dcdc";
+					regulator-boot-on;
+					regulator-always-on;
+				};
+
+				reg_vddd: regulator@2 {
+					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@3 {
+					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@4 {
+					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 25e25f8..378b1f2 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -1035,8 +1035,46 @@
 			};
 
 			power: power@80044000 {
+				compatible = "fsl,imx28-power", "syscon";
 				reg = <0x80044000 0x2000>;
-				status = "disabled";
+				interrupts = <6>;
+
+				dcdc: regulator@1 {
+					compatible = "fsl,imx28-dcdc";
+					regulator-name = "dcdc";
+					regulator-boot-on;
+					regulator-always-on;
+				};
+
+				reg_vddd: regulator@2 {
+					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@3 {
+					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@4 {
+					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 {
-- 
1.7.9.5