[PATCH 0/7] power: enable cpufreq-dt support for i.MX23/i.MX28

[PATCH 0/7] power: enable cpufreq-dt support for i.MX23/i.MX28

[Stefan Wahren]

This patch series adds cpufreq-dt support to Freescale i.MX23 and i.MX28.
It's the result of some discussions since August 2014 [1],[2],[3].

Summary
=======

Patch 1,2 - add driver for mxs power subsystem
Patch 3,4,5 - enable regulator support for i.MX23/i.MX28
Patch 6,7 - enable cpufreq-dt 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 MXS power subsystem is in the reference manual 
[4],[5].

Known issue
===========

The mxs clock driver provide the clock rate in Hz (454736842), but the 
operating points must be specified in kHz with less precision (454737).

So we get the following warning on boot [4]:

cpufreq: __cpufreq_add_dev: CPU0: Running at unlisted freq: 454736 KHz
cpufreq: __cpufreq_add_dev: CPU0: Unlisted initial frequency changed to: 454737 KHz

Changes
=======
- rebase on next-20150320
- complete rewrite of regulator ops based on helper functions and imx-bootlets
- move DC-DC handling from mxs_power into mxs-regulator
- make DC-DC switching frequency a device tree property
- fix NULL pointer dereference in mxs_power_remove()
- add i.MX23 support
- add cpufreq-dt support

[1] - http://www.spinics.net/lists/arm-kernel/msg356455.html
[2] - https://lkml.org/lkml/2014/9/26/932
[3] - http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/304688.html
[4] - http://cache.freescale.com/files/dsp/doc/ref_manual/IMX23RM.pdf
[5] - http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf
[6] - http://marc.info/?l=linux-pm&m=141225587213505&w=2

Stefan Wahren (7):
  DT: add binding for mxs power subsystem
  power: mxs_power: add driver for mxs power subsystem
  DT: add binding for MXS regulator
  regulator: add mxs regulator driver
  ARM: dts: enable regulator support for i.MX23/i.MX28
  ARM: mxs: register cpufreq-dt in pm init
  ARM: dts: add OPPs for i.MX23/i.MX28

 .../devicetree/bindings/power/mxs_power.txt        |   23 +
 .../bindings/regulator/mxs-regulator.txt           |   70 +++
 arch/arm/boot/dts/imx23.dtsi                       |   82 ++-
 arch/arm/boot/dts/imx28.dtsi                       |   82 ++-
 arch/arm/mach-mxs/pm.c                             |    6 +
 drivers/power/Kconfig                              |    8 +
 drivers/power/Makefile                             |    1 +
 drivers/power/mxs_power.c                          |  146 +++++
 drivers/regulator/Kconfig                          |    8 +
 drivers/regulator/Makefile                         |    1 +
 drivers/regulator/mxs-regulator.c                  |  624 ++++++++++++++++++++
 11 files changed, 1045 insertions(+), 6 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 0/7] power: enable cpufreq-dt support for i.MX23/i.MX28

[Stefan Wahren]

This patch series adds cpufreq-dt support to Freescale i.MX23 and i.MX28.
It's the result of some discussions since August 2014 [1],[2],[3].

Summary
=======

Patch 1,2 - add driver for mxs power subsystem
Patch 3,4,5 - enable regulator support for i.MX23/i.MX28
Patch 6,7 - enable cpufreq-dt 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 MXS power subsystem is in the reference manual 
[4],[5].

Known issue
===========

The mxs clock driver provide the clock rate in Hz (454736842), but the 
operating points must be specified in kHz with less precision (454737).

So we get the following warning on boot [4]:

cpufreq: __cpufreq_add_dev: CPU0: Running at unlisted freq: 454736 KHz
cpufreq: __cpufreq_add_dev: CPU0: Unlisted initial frequency changed to: 454737 KHz

Changes
=======
- rebase on next-20150320
- complete rewrite of regulator ops based on helper functions and imx-bootlets
- move DC-DC handling from mxs_power into mxs-regulator
- make DC-DC switching frequency a device tree property
- fix NULL pointer dereference in mxs_power_remove()
- add i.MX23 support
- add cpufreq-dt support

[1] - http://www.spinics.net/lists/arm-kernel/msg356455.html
[2] - https://lkml.org/lkml/2014/9/26/932
[3] - http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/304688.html
[4] - http://cache.freescale.com/files/dsp/doc/ref_manual/IMX23RM.pdf
[5] - http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf
[6] - http://marc.info/?l=linux-pm&m=141225587213505&w=2

Stefan Wahren (7):
  DT: add binding for mxs power subsystem
  power: mxs_power: add driver for mxs power subsystem
  DT: add binding for MXS regulator
  regulator: add mxs regulator driver
  ARM: dts: enable regulator support for i.MX23/i.MX28
  ARM: mxs: register cpufreq-dt in pm init
  ARM: dts: add OPPs for i.MX23/i.MX28

 .../devicetree/bindings/power/mxs_power.txt        |   23 +
 .../bindings/regulator/mxs-regulator.txt           |   70 +++
 arch/arm/boot/dts/imx23.dtsi                       |   82 ++-
 arch/arm/boot/dts/imx28.dtsi                       |   82 ++-
 arch/arm/mach-mxs/pm.c                             |    6 +
 drivers/power/Kconfig                              |    8 +
 drivers/power/Makefile                             |    1 +
 drivers/power/mxs_power.c                          |  146 +++++
 drivers/regulator/Kconfig                          |    8 +
 drivers/regulator/Makefile                         |    1 +
 drivers/regulator/mxs-regulator.c                  |  624 ++++++++++++++++++++
 11 files changed, 1045 insertions(+), 6 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 1/7] DT: add binding for mxs power subsystem

[Stefan Wahren]

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

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../devicetree/bindings/power/mxs_power.txt        |   23 ++++++++++++++++++++
 1 file changed, 23 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..aa9139b
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/mxs_power.txt
@@ -0,0 +1,23 @@
+Power subsystem for Freescale i.MX23/i.MX28
+
+Required properties:
+- compatible:
+  - "fsl,imx23-power" for i.MX23
+  - "fsl,imx28-power" for i.MX28
+- address-cells, size-cells, ranges: must be present as appropriate to
+  map children
+- 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";
+				#address-cells = <1>;
+				#size-cells = <1>;
+				reg = <0x80044000 0x2000>;
+				interrupts = <6>;
+				ranges;
+	}
-- 
1.7.9.5

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

[Stefan Wahren]

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

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 .../devicetree/bindings/power/mxs_power.txt        |   23 ++++++++++++++++++++
 1 file changed, 23 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..aa9139b
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/mxs_power.txt
@@ -0,0 +1,23 @@
+Power subsystem for Freescale i.MX23/i.MX28
+
+Required properties:
+- compatible:
+  - "fsl,imx23-power" for i.MX23
+  - "fsl,imx28-power" for i.MX28
+- address-cells, size-cells, ranges: must be present as appropriate to
+  map children
+- 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 at 80044000 {
+				compatible = "fsl,imx28-power";
+				#address-cells = <1>;
+				#size-cells = <1>;
+				reg = <0x80044000 0x2000>;
+				interrupts = <6>;
+				ranges;
+	}
-- 
1.7.9.5

[PATCH 2/7] 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 |  146 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 155 insertions(+)
 create mode 100644 drivers/power/mxs_power.c

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 4091fb0..b689017 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
+	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..f3428d3
--- /dev/null
+++ b/drivers/power/mxs_power.c
@@ -0,0 +1,146 @@
+/*
+ * 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/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/types.h>
+
+#define HW_POWER_CTRL_CLR	0x08
+
+#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 = 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 + HW_POWER_CTRL_CLR);
+
+	return of_platform_populate(np, NULL, NULL, dev);
+}
+
+static int mxs_power_remove(struct platform_device *pdev)
+{
+	struct mxs_power_data *data = platform_get_drvdata(pdev);
+
+	power_supply_unregister(data->ac);
+
+	return 0;
+}
+
+static struct platform_driver mxs_power_driver = {
+	.driver = {
+		.name	= "mxs_power",
+		.of_match_table = of_mxs_power_match,
+	},
+	.probe	= mxs_power_probe,
+	.remove = mxs_power_remove,
+};
+
+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

[PATCH 2/7] 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 |  146 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 155 insertions(+)
 create mode 100644 drivers/power/mxs_power.c

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 4091fb0..b689017 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
+	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..f3428d3
--- /dev/null
+++ b/drivers/power/mxs_power.c
@@ -0,0 +1,146 @@
+/*
+ * 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/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/types.h>
+
+#define HW_POWER_CTRL_CLR	0x08
+
+#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 = 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 + HW_POWER_CTRL_CLR);
+
+	return of_platform_populate(np, NULL, NULL, dev);
+}
+
+static int mxs_power_remove(struct platform_device *pdev)
+{
+	struct mxs_power_data *data = platform_get_drvdata(pdev);
+
+	power_supply_unregister(data->ac);
+
+	return 0;
+}
+
+static struct platform_driver mxs_power_driver = {
+	.driver = {
+		.name	= "mxs_power",
+		.of_match_table = of_mxs_power_match,
+	},
+	.probe	= mxs_power_probe,
+	.remove = mxs_power_remove,
+};
+
+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 2/7] power: mxs_power: add driver for mxs power subsystem

[Sebastian Reichel]

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

Hi,

On Sun, Mar 22, 2015 at 12:29:58AM +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.
> 
> [...]
>
> +config MXS_POWER
> +	tristate "Freescale MXS power subsystem support"
> +	depends on ARCH_MXS

please add "|| 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.
>
> [...]
>
> +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 = power_supply_register(dev, &ac_desc, &psy_cfg);
> +	if (IS_ERR(data->ac))
> +		return PTR_ERR(data->ac);

You can use devm_power_supply_register to simplify the driver
a bit more.

> +	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 + HW_POWER_CTRL_CLR);
> +
> +	return of_platform_populate(np, NULL, NULL, dev);
> +}
> +
> [...]

-- Sebastian

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

[PATCH 2/7] power: mxs_power: add driver for mxs power subsystem

[Sebastian Reichel]

Hi,

On Sun, Mar 22, 2015 at 12:29:58AM +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.
> 
> [...]
>
> +config MXS_POWER
> +	tristate "Freescale MXS power subsystem support"
> +	depends on ARCH_MXS

please add "|| 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.
>
> [...]
>
> +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 = power_supply_register(dev, &ac_desc, &psy_cfg);
> +	if (IS_ERR(data->ac))
> +		return PTR_ERR(data->ac);

You can use devm_power_supply_register to simplify the driver
a bit more.

> +	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 + HW_POWER_CTRL_CLR);
> +
> +	return of_platform_populate(np, NULL, NULL, dev);
> +}
> +
> [...]

-- Sebastian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150322/76b83142/attachment.sig>

[PATCH 3/7] DT: add binding for MXS regulator

[Stefan Wahren]

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

Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
---
 .../bindings/regulator/mxs-regulator.txt           |   70 ++++++++++++++++++++
 1 file changed, 70 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..88452d2
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/mxs-regulator.txt
@@ -0,0 +1,70 @@
+Regulators for Freescale i.MX23/i.MX28
+
+Required properties:
+For imx23 regulators
+- compatible:
+  - "fsl,imx23-dcdc" for DC-DC converter
+  - "fsl,imx23-vddd" for VDDD linear regulator
+  - "fsl,imx23-vdda" for VDDA linear regulator
+  - "fsl,imx23-vddio" for VDDIO linear regulator
+For imx28 regulators
+- compatible:
+  - "fsl,imx28-dcdc" for DC-DC converter
+  - "fsl,imx28-vddd" for VDDD linear regulator
+  - "fsl,imx28-vdda" for VDDA linear regulator
+  - "fsl,imx28-vddio" for VDDIO linear regulator
+- reg: Address and length of the register set for the device. It contains
+  the information of registers in the same order as described by reg-names
+- reg-names: Should contain the reg names
+  - "base-address"	- contains base address of regulator
+  - "status-address"	- contains status address of regulator
+  - "v5ctrl-address"	- contains 5V control address of DC-DC converter
+			  (required only for linear regulators)
+  - "misc-address"      - contains misc control address of DC-DC converter
+			  (required only for DC-DC converter)
+
+Optional properties:
+- switching-frequency: switching frequency of the DC-DC converter in Hz.
+  Possible values are <19200000>, <22000000> or <24000000> (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";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0x80044000 0x2000>;
+		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;
+			switching-frequency = <24000000>;
+		};
+
+		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>;
+		};
+	};
+
-- 
1.7.9.5

--
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 3/7] DT: add binding for MXS regulator

[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           |   70 ++++++++++++++++++++
 1 file changed, 70 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..88452d2
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/mxs-regulator.txt
@@ -0,0 +1,70 @@
+Regulators for Freescale i.MX23/i.MX28
+
+Required properties:
+For imx23 regulators
+- compatible:
+  - "fsl,imx23-dcdc" for DC-DC converter
+  - "fsl,imx23-vddd" for VDDD linear regulator
+  - "fsl,imx23-vdda" for VDDA linear regulator
+  - "fsl,imx23-vddio" for VDDIO linear regulator
+For imx28 regulators
+- compatible:
+  - "fsl,imx28-dcdc" for DC-DC converter
+  - "fsl,imx28-vddd" for VDDD linear regulator
+  - "fsl,imx28-vdda" for VDDA linear regulator
+  - "fsl,imx28-vddio" for VDDIO linear regulator
+- reg: Address and length of the register set for the device. It contains
+  the information of registers in the same order as described by reg-names
+- reg-names: Should contain the reg names
+  - "base-address"	- contains base address of regulator
+  - "status-address"	- contains status address of regulator
+  - "v5ctrl-address"	- contains 5V control address of DC-DC converter
+			  (required only for linear regulators)
+  - "misc-address"      - contains misc control address of DC-DC converter
+			  (required only for DC-DC converter)
+
+Optional properties:
+- switching-frequency: switching frequency of the DC-DC converter in Hz.
+  Possible values are <19200000>, <22000000> or <24000000> (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 at 80044000 {
+		compatible = "fsl,imx28-power";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0x80044000 0x2000>;
+		interrupts = <6>;
+		ranges;
+
+		dcdc: regulator at 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;
+			switching-frequency = <24000000>;
+		};
+
+		reg_vddd: regulator at 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>;
+		};
+	};
+
-- 
1.7.9.5

[PATCH 4/7] regulator: add mxs 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 |  624 +++++++++++++++++++++++++++++++++++++
 3 files changed, 633 insertions(+)
 create mode 100644 drivers/regulator/mxs-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index a6f116a..85504ab 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
+	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..93a0d1d
--- /dev/null
+++ b/drivers/regulator/mxs-regulator.c
@@ -0,0 +1,624 @@
+/*
+ * Freescale MXS 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/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/slab.h>
+
+#define BM_POWER_LINREG_OFFSET_DCDC_MODE	BIT(1)
+
+/* 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
+
+#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 SHIFT_FREQSEL			4
+
+#define BM_POWER_MISC_FREQSEL		(7 << SHIFT_FREQSEL)
+
+#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)
+
+#define MXS_DCDC	1
+#define MXS_VDDIO	2
+#define MXS_VDDA	3
+#define MXS_VDDD	4
+
+struct mxs_dcdc {
+	struct regulator_desc desc;
+
+	void __iomem *base_addr;
+	void __iomem *status_addr;
+	void __iomem *misc_addr;
+};
+
+struct mxs_ldo {
+	struct regulator_desc desc;
+	unsigned int disable_fet_mask;
+	unsigned int linreg_offset_mask;
+	u8 linreg_offset_shift;
+	u8 (*get_power_source)(struct regulator_dev *);
+
+	void __iomem *base_addr;
+	void __iomem *status_addr;
+	void __iomem *v5ctrl_addr;
+};
+
+static inline u8 get_linreg_offset(struct mxs_ldo *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_ldo *ldo = rdev_get_drvdata(reg);
+	u32 v5ctrl, status, base;
+	u8 offset;
+
+	v5ctrl = readl(ldo->v5ctrl_addr);
+	status = readl(ldo->status_addr);
+	base = readl(ldo->base_addr);
+	offset = get_linreg_offset(ldo, base);
+
+	if (status & BM_POWER_STS_VBUSVALID0_STATUS) {
+		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) {
+			if (offset & BM_POWER_LINREG_OFFSET_DCDC_MODE)
+				return HW_POWER_DCDC_LINREG_ON;
+		} else {
+			if (!(offset & BM_POWER_LINREG_OFFSET_DCDC_MODE))
+				return HW_POWER_LINREG_DCDC_OFF;
+		}
+	} else {
+		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_ldo *ldo = rdev_get_drvdata(reg);
+	struct regulator_desc *desc = &ldo->desc;
+	u32 v5ctrl, status, base;
+	u8 offset;
+
+	v5ctrl = readl(ldo->v5ctrl_addr);
+	status = readl(ldo->status_addr);
+	base = readl(ldo->base_addr);
+	offset = get_linreg_offset(ldo, base);
+
+	if (base & ldo->disable_fet_mask) {
+		if (status & BM_POWER_STS_VBUSVALID0_STATUS)
+			return HW_POWER_EXTERNAL_SOURCE_5V;
+
+		if (!(offset & BM_POWER_LINREG_OFFSET_DCDC_MODE))
+			return HW_POWER_LINREG_DCDC_OFF;
+	}
+
+	if (status & BM_POWER_STS_VBUSVALID0_STATUS) {
+		if (v5ctrl & BM_POWER_5VCTRL_ENABLE_DCDC)
+			return HW_POWER_DCDC_LINREG_ON;
+
+		return HW_POWER_LINREG_DCDC_OFF;
+	}
+
+	if (offset & BM_POWER_LINREG_OFFSET_DCDC_MODE) {
+		if (base & desc->enable_mask)
+			return HW_POWER_DCDC_LINREG_ON;
+
+		return HW_POWER_DCDC_LINREG_OFF;
+	}
+
+	return HW_POWER_UNKNOWN_SOURCE;
+}
+
+int get_dcdc_clk_freq(struct mxs_dcdc *dcdc)
+{
+	int ret = -EINVAL;
+	u32 val;
+
+	val = readl(dcdc->misc_addr);
+
+	/* XTAL source */
+	if ((val & HW_POWER_MISC_SEL_PLLCLK) == 0)
+		return 24000;
+
+	switch ((val & BM_POWER_MISC_FREQSEL) >> SHIFT_FREQSEL) {
+	case HW_POWER_MISC_FREQSEL_20000_KHZ:
+		ret = 20000;
+		break;
+	case HW_POWER_MISC_FREQSEL_24000_KHZ:
+		ret = 24000;
+		break;
+	case HW_POWER_MISC_FREQSEL_19200_KHZ:
+		ret = 19200;
+		break;
+	}
+
+	return ret;
+}
+
+int set_dcdc_clk_freq(struct mxs_dcdc *dcdc, int khz)
+{
+	u32 val;
+	int ret = 0;
+
+	val = readl(dcdc->misc_addr);
+
+	val &= ~BM_POWER_MISC_FREQSEL;
+	val &= ~HW_POWER_MISC_SEL_PLLCLK;
+
+	/* Accept only values recommend by Freescale */
+	switch (khz) {
+	case 19200:
+		val |= HW_POWER_MISC_FREQSEL_19200_KHZ << SHIFT_FREQSEL;
+		break;
+	case 20000:
+		val |= HW_POWER_MISC_FREQSEL_20000_KHZ << SHIFT_FREQSEL;
+		break;
+	case 24000:
+		val |= HW_POWER_MISC_FREQSEL_24000_KHZ << SHIFT_FREQSEL;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	if (ret)
+		return ret;
+
+	/* First program FREQSEL */
+	writel(val, dcdc->misc_addr);
+
+	/* then set PLL as clk for DC-DC converter */
+	writel(val | HW_POWER_MISC_SEL_PLLCLK, dcdc->misc_addr);
+
+	return 0;
+}
+
+static int mxs_ldo_set_voltage_sel(struct regulator_dev *reg, unsigned sel)
+{
+	struct mxs_ldo *ldo = rdev_get_drvdata(reg);
+	struct regulator_desc *desc = &ldo->desc;
+	unsigned long start;
+	u32 regs;
+	int uV;
+	u8 power_source = HW_POWER_UNKNOWN_SOURCE;
+
+	uV = regulator_list_voltage_linear(reg, sel);
+
+	regs = (readl(ldo->base_addr) & ~desc->vsel_mask);
+	writel(sel | regs, ldo->base_addr);
+
+	if (ldo->get_power_source)
+		power_source = ldo->get_power_source(reg);
+
+	switch (power_source) {
+	case HW_POWER_LINREG_DCDC_OFF:
+	case HW_POWER_LINREG_DCDC_READY:
+	case HW_POWER_EXTERNAL_SOURCE_5V:
+		usleep_range(1000, 2000);
+		return 0;
+	}
+
+	usleep_range(15, 20);
+	start = jiffies;
+	while (1) {
+		if (readl(ldo->status_addr) & BM_POWER_STS_DC_OK)
+			return 0;
+
+		if (time_after(jiffies, start +	msecs_to_jiffies(20)))
+			break;
+
+		schedule();
+	}
+
+	dev_warn_ratelimited(&reg->dev, "%s: timeout status=0x%08x\n",
+			     __func__, readl(ldo->status_addr));
+
+	return -ETIMEDOUT;
+}
+
+static int mxs_ldo_get_voltage_sel(struct regulator_dev *reg)
+{
+	struct mxs_ldo *ldo = rdev_get_drvdata(reg);
+	struct regulator_desc *desc = &ldo->desc;
+	int ret, uV;
+
+	ret = readl(ldo->base_addr) & desc->vsel_mask;
+	uV = regulator_list_voltage_linear(reg, ret);
+
+	return ret;
+}
+
+static int mxs_dcdc_is_enabled(struct regulator_dev *reg)
+{
+	struct mxs_dcdc *dcdc = rdev_get_drvdata(reg);
+
+	if (readl(dcdc->base_addr) & BM_POWER_5VCTRL_ENABLE_DCDC)
+		return 1;
+
+	return 0;
+}
+
+static int mxs_ldo_is_enabled(struct regulator_dev *reg)
+{
+	struct mxs_ldo *ldo = rdev_get_drvdata(reg);
+	u8 power_source = HW_POWER_UNKNOWN_SOURCE;
+
+	if (ldo->get_power_source)
+		power_source = ldo->get_power_source(reg);
+
+	switch (power_source) {
+	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		= mxs_dcdc_is_enabled,
+};
+
+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	= mxs_ldo_get_voltage_sel,
+	.is_enabled		= mxs_ldo_is_enabled,
+};
+
+static const struct mxs_dcdc mxs_info_dcdc = {
+	.desc = {
+		.name = "dcdc",
+		.id = MXS_DCDC,
+		.type = REGULATOR_VOLTAGE,
+		.owner = THIS_MODULE,
+		.ops = &mxs_dcdc_ops,
+		.enable_mask = (1 << 0),
+	},
+};
+
+static const struct mxs_ldo 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_mask = 0x1f,
+		.ops = &mxs_ldo_ops,
+	},
+	.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_ldo 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_mask = 0x1f,
+		.ops = &mxs_ldo_ops,
+	},
+	.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_ldo 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_mask = 0x1f,
+		.ops = &mxs_ldo_ops,
+		.enable_mask = (1 << 17),
+	},
+	.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_ldo 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_mask = 0x1f,
+		.ops = &mxs_ldo_ops,
+		.enable_mask = (1 << 21),
+	},
+	.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);
+
+struct regulator_dev *mxs_dcdc_register(struct platform_device *pdev, const void *data)
+{
+	struct device *dev = &pdev->dev;
+	struct mxs_dcdc *dcdc;
+	struct regulator_init_data *initdata;
+	struct regulator_config config = { };
+	struct resource *res;
+	char *pname;
+	u32 dcdc_clk_freq;
+
+	dcdc = devm_kmemdup(dev, data, sizeof(*dcdc), GFP_KERNEL);
+	if (!dcdc)
+		return NULL;
+
+	pname = "base-address";
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
+	if (!res) {
+		dev_err(dev, "Missing '%s' IO resource\n", pname);
+		return NULL;
+	}
+	dcdc->base_addr = devm_ioremap_nocache(dev, res->start,
+						 resource_size(res));
+	if (IS_ERR(dcdc->base_addr))
+		return NULL;
+
+	/* status register is shared between the regulators */
+	pname = "status-address";
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
+	if (!res) {
+		dev_err(dev, "Missing '%s' IO resource\n", pname);
+		return NULL;
+	}
+	dcdc->status_addr = devm_ioremap_nocache(dev, res->start,
+						 resource_size(res));
+	if (IS_ERR(dcdc->status_addr))
+		return NULL;
+
+	pname = "misc-address";
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
+	if (!res) {
+		dev_err(dev, "Missing '%s' IO resource\n", pname);
+		return NULL;
+	}
+	dcdc->misc_addr = devm_ioremap_nocache(dev, res->start,
+						 resource_size(res));
+	if (IS_ERR(dcdc->misc_addr))
+		return NULL;
+
+	initdata = of_get_regulator_init_data(dev, dev->of_node, &dcdc->desc);
+	if (!initdata)
+		return NULL;
+
+	config.driver_data = dcdc;
+	config.dev = dev;
+	config.init_data = initdata;
+	config.of_node = dev->of_node;
+
+	pname = "switching-frequency";
+	if (!of_property_read_u32(dev->of_node, pname, &dcdc_clk_freq))
+		set_dcdc_clk_freq(dcdc, dcdc_clk_freq / 1000);
+
+	dcdc_clk_freq = get_dcdc_clk_freq(dcdc);
+	dev_info(dev, "DCDC clock freq: %d kHz\n", dcdc_clk_freq);
+
+	return devm_regulator_register(dev, &dcdc->desc, &config);
+}
+
+struct regulator_dev *mxs_ldo_register(struct platform_device *pdev, const void *data)
+{
+	struct device *dev = &pdev->dev;
+	struct mxs_ldo *ldo;
+	struct regulator_init_data *initdata;
+	struct regulator_config config = { };
+	struct resource *res;
+	char *pname;
+
+	ldo = devm_kmemdup(dev, data, sizeof(*ldo), GFP_KERNEL);
+	if (!ldo)
+		return NULL;
+
+	pname = "base-address";
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
+	if (!res) {
+		dev_err(dev, "Missing '%s' IO resource\n", pname);
+		return NULL;
+	}
+	ldo->base_addr = devm_ioremap_nocache(dev, res->start,
+						 resource_size(res));
+	if (IS_ERR(ldo->base_addr))
+		return NULL;
+
+	/* status register is shared between the regulators */
+	pname = "status-address";
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
+	if (!res) {
+		dev_err(dev, "Missing '%s' IO resource\n", pname);
+		return NULL;
+	}
+	ldo->status_addr = devm_ioremap_nocache(dev, res->start,
+						 resource_size(res));
+	if (IS_ERR(ldo->status_addr))
+		return NULL;
+
+	pname = "v5ctrl-address";
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
+	if (!res) {
+		dev_err(dev, "Missing '%s' IO resource\n", pname);
+		return NULL;
+	}
+	ldo->v5ctrl_addr = devm_ioremap_nocache(dev, res->start,
+						 resource_size(res));
+	if (IS_ERR(ldo->v5ctrl_addr))
+		return NULL;
+
+	initdata = of_get_regulator_init_data(dev, dev->of_node, &ldo->desc);
+	if (!initdata)
+		return NULL;
+
+	config.dev = dev;
+	config.init_data = initdata;
+	config.driver_data = ldo;
+	config.of_node = dev->of_node;
+
+	return devm_regulator_register(dev, &ldo->desc, &config);
+}
+
+static int mxs_regulator_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct of_device_id *match;
+	struct regulator_dev *rdev = NULL;
+	int ret = 0;
+
+	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;
+	}
+
+	if ((strcmp(match->compatible, "fsl,imx23-dcdc") == 0) ||
+	    (strcmp(match->compatible, "fsl,imx28-dcdc") == 0)) {
+		rdev = mxs_dcdc_register(pdev, match->data);
+	} else {
+		rdev = mxs_ldo_register(pdev, match->data);
+	}
+
+	if (IS_ERR(rdev)) {
+		ret = PTR_ERR(rdev);
+		dev_err(dev, "%s: failed to register regulator(%d)\n",
+			__func__, ret);
+		return ret;
+	}
+
+	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

[PATCH 4/7] regulator: add mxs 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 |  624 +++++++++++++++++++++++++++++++++++++
 3 files changed, 633 insertions(+)
 create mode 100644 drivers/regulator/mxs-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index a6f116a..85504ab 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
+	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..93a0d1d
--- /dev/null
+++ b/drivers/regulator/mxs-regulator.c
@@ -0,0 +1,624 @@
+/*
+ * Freescale MXS 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/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/slab.h>
+
+#define BM_POWER_LINREG_OFFSET_DCDC_MODE	BIT(1)
+
+/* 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
+
+#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 SHIFT_FREQSEL			4
+
+#define BM_POWER_MISC_FREQSEL		(7 << SHIFT_FREQSEL)
+
+#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)
+
+#define MXS_DCDC	1
+#define MXS_VDDIO	2
+#define MXS_VDDA	3
+#define MXS_VDDD	4
+
+struct mxs_dcdc {
+	struct regulator_desc desc;
+
+	void __iomem *base_addr;
+	void __iomem *status_addr;
+	void __iomem *misc_addr;
+};
+
+struct mxs_ldo {
+	struct regulator_desc desc;
+	unsigned int disable_fet_mask;
+	unsigned int linreg_offset_mask;
+	u8 linreg_offset_shift;
+	u8 (*get_power_source)(struct regulator_dev *);
+
+	void __iomem *base_addr;
+	void __iomem *status_addr;
+	void __iomem *v5ctrl_addr;
+};
+
+static inline u8 get_linreg_offset(struct mxs_ldo *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_ldo *ldo = rdev_get_drvdata(reg);
+	u32 v5ctrl, status, base;
+	u8 offset;
+
+	v5ctrl = readl(ldo->v5ctrl_addr);
+	status = readl(ldo->status_addr);
+	base = readl(ldo->base_addr);
+	offset = get_linreg_offset(ldo, base);
+
+	if (status & BM_POWER_STS_VBUSVALID0_STATUS) {
+		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) {
+			if (offset & BM_POWER_LINREG_OFFSET_DCDC_MODE)
+				return HW_POWER_DCDC_LINREG_ON;
+		} else {
+			if (!(offset & BM_POWER_LINREG_OFFSET_DCDC_MODE))
+				return HW_POWER_LINREG_DCDC_OFF;
+		}
+	} else {
+		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_ldo *ldo = rdev_get_drvdata(reg);
+	struct regulator_desc *desc = &ldo->desc;
+	u32 v5ctrl, status, base;
+	u8 offset;
+
+	v5ctrl = readl(ldo->v5ctrl_addr);
+	status = readl(ldo->status_addr);
+	base = readl(ldo->base_addr);
+	offset = get_linreg_offset(ldo, base);
+
+	if (base & ldo->disable_fet_mask) {
+		if (status & BM_POWER_STS_VBUSVALID0_STATUS)
+			return HW_POWER_EXTERNAL_SOURCE_5V;
+
+		if (!(offset & BM_POWER_LINREG_OFFSET_DCDC_MODE))
+			return HW_POWER_LINREG_DCDC_OFF;
+	}
+
+	if (status & BM_POWER_STS_VBUSVALID0_STATUS) {
+		if (v5ctrl & BM_POWER_5VCTRL_ENABLE_DCDC)
+			return HW_POWER_DCDC_LINREG_ON;
+
+		return HW_POWER_LINREG_DCDC_OFF;
+	}
+
+	if (offset & BM_POWER_LINREG_OFFSET_DCDC_MODE) {
+		if (base & desc->enable_mask)
+			return HW_POWER_DCDC_LINREG_ON;
+
+		return HW_POWER_DCDC_LINREG_OFF;
+	}
+
+	return HW_POWER_UNKNOWN_SOURCE;
+}
+
+int get_dcdc_clk_freq(struct mxs_dcdc *dcdc)
+{
+	int ret = -EINVAL;
+	u32 val;
+
+	val = readl(dcdc->misc_addr);
+
+	/* XTAL source */
+	if ((val & HW_POWER_MISC_SEL_PLLCLK) == 0)
+		return 24000;
+
+	switch ((val & BM_POWER_MISC_FREQSEL) >> SHIFT_FREQSEL) {
+	case HW_POWER_MISC_FREQSEL_20000_KHZ:
+		ret = 20000;
+		break;
+	case HW_POWER_MISC_FREQSEL_24000_KHZ:
+		ret = 24000;
+		break;
+	case HW_POWER_MISC_FREQSEL_19200_KHZ:
+		ret = 19200;
+		break;
+	}
+
+	return ret;
+}
+
+int set_dcdc_clk_freq(struct mxs_dcdc *dcdc, int khz)
+{
+	u32 val;
+	int ret = 0;
+
+	val = readl(dcdc->misc_addr);
+
+	val &= ~BM_POWER_MISC_FREQSEL;
+	val &= ~HW_POWER_MISC_SEL_PLLCLK;
+
+	/* Accept only values recommend by Freescale */
+	switch (khz) {
+	case 19200:
+		val |= HW_POWER_MISC_FREQSEL_19200_KHZ << SHIFT_FREQSEL;
+		break;
+	case 20000:
+		val |= HW_POWER_MISC_FREQSEL_20000_KHZ << SHIFT_FREQSEL;
+		break;
+	case 24000:
+		val |= HW_POWER_MISC_FREQSEL_24000_KHZ << SHIFT_FREQSEL;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	if (ret)
+		return ret;
+
+	/* First program FREQSEL */
+	writel(val, dcdc->misc_addr);
+
+	/* then set PLL as clk for DC-DC converter */
+	writel(val | HW_POWER_MISC_SEL_PLLCLK, dcdc->misc_addr);
+
+	return 0;
+}
+
+static int mxs_ldo_set_voltage_sel(struct regulator_dev *reg, unsigned sel)
+{
+	struct mxs_ldo *ldo = rdev_get_drvdata(reg);
+	struct regulator_desc *desc = &ldo->desc;
+	unsigned long start;
+	u32 regs;
+	int uV;
+	u8 power_source = HW_POWER_UNKNOWN_SOURCE;
+
+	uV = regulator_list_voltage_linear(reg, sel);
+
+	regs = (readl(ldo->base_addr) & ~desc->vsel_mask);
+	writel(sel | regs, ldo->base_addr);
+
+	if (ldo->get_power_source)
+		power_source = ldo->get_power_source(reg);
+
+	switch (power_source) {
+	case HW_POWER_LINREG_DCDC_OFF:
+	case HW_POWER_LINREG_DCDC_READY:
+	case HW_POWER_EXTERNAL_SOURCE_5V:
+		usleep_range(1000, 2000);
+		return 0;
+	}
+
+	usleep_range(15, 20);
+	start = jiffies;
+	while (1) {
+		if (readl(ldo->status_addr) & BM_POWER_STS_DC_OK)
+			return 0;
+
+		if (time_after(jiffies, start +	msecs_to_jiffies(20)))
+			break;
+
+		schedule();
+	}
+
+	dev_warn_ratelimited(&reg->dev, "%s: timeout status=0x%08x\n",
+			     __func__, readl(ldo->status_addr));
+
+	return -ETIMEDOUT;
+}
+
+static int mxs_ldo_get_voltage_sel(struct regulator_dev *reg)
+{
+	struct mxs_ldo *ldo = rdev_get_drvdata(reg);
+	struct regulator_desc *desc = &ldo->desc;
+	int ret, uV;
+
+	ret = readl(ldo->base_addr) & desc->vsel_mask;
+	uV = regulator_list_voltage_linear(reg, ret);
+
+	return ret;
+}
+
+static int mxs_dcdc_is_enabled(struct regulator_dev *reg)
+{
+	struct mxs_dcdc *dcdc = rdev_get_drvdata(reg);
+
+	if (readl(dcdc->base_addr) & BM_POWER_5VCTRL_ENABLE_DCDC)
+		return 1;
+
+	return 0;
+}
+
+static int mxs_ldo_is_enabled(struct regulator_dev *reg)
+{
+	struct mxs_ldo *ldo = rdev_get_drvdata(reg);
+	u8 power_source = HW_POWER_UNKNOWN_SOURCE;
+
+	if (ldo->get_power_source)
+		power_source = ldo->get_power_source(reg);
+
+	switch (power_source) {
+	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		= mxs_dcdc_is_enabled,
+};
+
+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	= mxs_ldo_get_voltage_sel,
+	.is_enabled		= mxs_ldo_is_enabled,
+};
+
+static const struct mxs_dcdc mxs_info_dcdc = {
+	.desc = {
+		.name = "dcdc",
+		.id = MXS_DCDC,
+		.type = REGULATOR_VOLTAGE,
+		.owner = THIS_MODULE,
+		.ops = &mxs_dcdc_ops,
+		.enable_mask = (1 << 0),
+	},
+};
+
+static const struct mxs_ldo 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_mask = 0x1f,
+		.ops = &mxs_ldo_ops,
+	},
+	.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_ldo 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_mask = 0x1f,
+		.ops = &mxs_ldo_ops,
+	},
+	.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_ldo 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_mask = 0x1f,
+		.ops = &mxs_ldo_ops,
+		.enable_mask = (1 << 17),
+	},
+	.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_ldo 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_mask = 0x1f,
+		.ops = &mxs_ldo_ops,
+		.enable_mask = (1 << 21),
+	},
+	.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);
+
+struct regulator_dev *mxs_dcdc_register(struct platform_device *pdev, const void *data)
+{
+	struct device *dev = &pdev->dev;
+	struct mxs_dcdc *dcdc;
+	struct regulator_init_data *initdata;
+	struct regulator_config config = { };
+	struct resource *res;
+	char *pname;
+	u32 dcdc_clk_freq;
+
+	dcdc = devm_kmemdup(dev, data, sizeof(*dcdc), GFP_KERNEL);
+	if (!dcdc)
+		return NULL;
+
+	pname = "base-address";
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
+	if (!res) {
+		dev_err(dev, "Missing '%s' IO resource\n", pname);
+		return NULL;
+	}
+	dcdc->base_addr = devm_ioremap_nocache(dev, res->start,
+						 resource_size(res));
+	if (IS_ERR(dcdc->base_addr))
+		return NULL;
+
+	/* status register is shared between the regulators */
+	pname = "status-address";
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
+	if (!res) {
+		dev_err(dev, "Missing '%s' IO resource\n", pname);
+		return NULL;
+	}
+	dcdc->status_addr = devm_ioremap_nocache(dev, res->start,
+						 resource_size(res));
+	if (IS_ERR(dcdc->status_addr))
+		return NULL;
+
+	pname = "misc-address";
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
+	if (!res) {
+		dev_err(dev, "Missing '%s' IO resource\n", pname);
+		return NULL;
+	}
+	dcdc->misc_addr = devm_ioremap_nocache(dev, res->start,
+						 resource_size(res));
+	if (IS_ERR(dcdc->misc_addr))
+		return NULL;
+
+	initdata = of_get_regulator_init_data(dev, dev->of_node, &dcdc->desc);
+	if (!initdata)
+		return NULL;
+
+	config.driver_data = dcdc;
+	config.dev = dev;
+	config.init_data = initdata;
+	config.of_node = dev->of_node;
+
+	pname = "switching-frequency";
+	if (!of_property_read_u32(dev->of_node, pname, &dcdc_clk_freq))
+		set_dcdc_clk_freq(dcdc, dcdc_clk_freq / 1000);
+
+	dcdc_clk_freq = get_dcdc_clk_freq(dcdc);
+	dev_info(dev, "DCDC clock freq: %d kHz\n", dcdc_clk_freq);
+
+	return devm_regulator_register(dev, &dcdc->desc, &config);
+}
+
+struct regulator_dev *mxs_ldo_register(struct platform_device *pdev, const void *data)
+{
+	struct device *dev = &pdev->dev;
+	struct mxs_ldo *ldo;
+	struct regulator_init_data *initdata;
+	struct regulator_config config = { };
+	struct resource *res;
+	char *pname;
+
+	ldo = devm_kmemdup(dev, data, sizeof(*ldo), GFP_KERNEL);
+	if (!ldo)
+		return NULL;
+
+	pname = "base-address";
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
+	if (!res) {
+		dev_err(dev, "Missing '%s' IO resource\n", pname);
+		return NULL;
+	}
+	ldo->base_addr = devm_ioremap_nocache(dev, res->start,
+						 resource_size(res));
+	if (IS_ERR(ldo->base_addr))
+		return NULL;
+
+	/* status register is shared between the regulators */
+	pname = "status-address";
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
+	if (!res) {
+		dev_err(dev, "Missing '%s' IO resource\n", pname);
+		return NULL;
+	}
+	ldo->status_addr = devm_ioremap_nocache(dev, res->start,
+						 resource_size(res));
+	if (IS_ERR(ldo->status_addr))
+		return NULL;
+
+	pname = "v5ctrl-address";
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, pname);
+	if (!res) {
+		dev_err(dev, "Missing '%s' IO resource\n", pname);
+		return NULL;
+	}
+	ldo->v5ctrl_addr = devm_ioremap_nocache(dev, res->start,
+						 resource_size(res));
+	if (IS_ERR(ldo->v5ctrl_addr))
+		return NULL;
+
+	initdata = of_get_regulator_init_data(dev, dev->of_node, &ldo->desc);
+	if (!initdata)
+		return NULL;
+
+	config.dev = dev;
+	config.init_data = initdata;
+	config.driver_data = ldo;
+	config.of_node = dev->of_node;
+
+	return devm_regulator_register(dev, &ldo->desc, &config);
+}
+
+static int mxs_regulator_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct of_device_id *match;
+	struct regulator_dev *rdev = NULL;
+	int ret = 0;
+
+	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;
+	}
+
+	if ((strcmp(match->compatible, "fsl,imx23-dcdc") == 0) ||
+	    (strcmp(match->compatible, "fsl,imx28-dcdc") == 0)) {
+		rdev = mxs_dcdc_register(pdev, match->data);
+	} else {
+		rdev = mxs_ldo_register(pdev, match->data);
+	}
+
+	if (IS_ERR(rdev)) {
+		ret = PTR_ERR(rdev);
+		dev_err(dev, "%s: failed to register regulator(%d)\n",
+			__func__, ret);
+		return ret;
+	}
+
+	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 4/7] regulator: add mxs regulator driver

[Mark Brown]

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

On Sun, Mar 22, 2015 at 12:30:00AM +0000, Stefan Wahren wrote:

> +	/* Accept only values recommend by Freescale */
> +	switch (khz) {
> +	case 19200:
> +		val |= HW_POWER_MISC_FREQSEL_19200_KHZ << SHIFT_FREQSEL;
> +		break;
> +	case 20000:
> +		val |= HW_POWER_MISC_FREQSEL_20000_KHZ << SHIFT_FREQSEL;
> +		break;
> +	case 24000:
> +		val |= HW_POWER_MISC_FREQSEL_24000_KHZ << SHIFT_FREQSEL;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +	if (ret)
> +		return ret;

Just return directly, it'd be a bit easier to follow.  It'd also be
better to print an error message saying we're rejecting the value - that
will make it easier for someone writing a DT to figure out that they
have made a typo or whatever.

> +static int mxs_ldo_set_voltage_sel(struct regulator_dev *reg, unsigned sel)
> +{
> +	struct mxs_ldo *ldo = rdev_get_drvdata(reg);
> +	struct regulator_desc *desc = &ldo->desc;
> +	unsigned long start;
> +	u32 regs;
> +	int uV;
> +	u8 power_source = HW_POWER_UNKNOWN_SOURCE;
> +
> +	uV = regulator_list_voltage_linear(reg, sel);

This would be strange execpt uV doesn't seem to be referenced at all so
it could be removed instead.

> +	if (ldo->get_power_source)
> +		power_source = ldo->get_power_source(reg);
> +
> +	switch (power_source) {
> +	case HW_POWER_LINREG_DCDC_OFF:
> +	case HW_POWER_LINREG_DCDC_READY:
> +	case HW_POWER_EXTERNAL_SOURCE_5V:
> +		usleep_range(1000, 2000);
> +		return 0;
> +	}

I'd expect the switch to be in the if here?

> +
> +	usleep_range(15, 20);
> +	start = jiffies;
> +	while (1) {
> +		if (readl(ldo->status_addr) & BM_POWER_STS_DC_OK)
> +			return 0;
> +
> +		if (time_after(jiffies, start +	msecs_to_jiffies(20)))
> +			break;
> +
> +		schedule();
> +	}

So, this isn't actually quite a busy wait because we do a schedule()
rather than a cpu_relax() but still it could devolve into that - 20ms
seems a long time to burn doing that.  If we're expecting this to finish
very quickly can we do an initial busy wait then fall back to something
with an actual sleep or soemthing?

> +static int mxs_ldo_get_voltage_sel(struct regulator_dev *reg)
> +{
> +	struct mxs_ldo *ldo = rdev_get_drvdata(reg);
> +	struct regulator_desc *desc = &ldo->desc;
> +	int ret, uV;
> +
> +	ret = readl(ldo->base_addr) & desc->vsel_mask;
> +	uV = regulator_list_voltage_linear(reg, ret);
> +
> +	return ret;
> +}

Again with the uV lookup...

> +static int mxs_ldo_is_enabled(struct regulator_dev *reg)
> +{
> +	struct mxs_ldo *ldo = rdev_get_drvdata(reg);
> +	u8 power_source = HW_POWER_UNKNOWN_SOURCE;
> +
> +	if (ldo->get_power_source)
> +		power_source = ldo->get_power_source(reg);
> +
> +	switch (power_source) {
> +	case HW_POWER_LINREG_DCDC_OFF:
> +	case HW_POWER_LINREG_DCDC_READY:
> +	case HW_POWER_DCDC_LINREG_ON:
> +		return 1;
> +	}
> +
> +	return 0;
> +}

Some comments explaining what the logic here is might be helpful, it's
all a bit surprising.

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

[PATCH 4/7] regulator: add mxs regulator driver

[Mark Brown]

On Sun, Mar 22, 2015 at 12:30:00AM +0000, Stefan Wahren wrote:

> +	/* Accept only values recommend by Freescale */
> +	switch (khz) {
> +	case 19200:
> +		val |= HW_POWER_MISC_FREQSEL_19200_KHZ << SHIFT_FREQSEL;
> +		break;
> +	case 20000:
> +		val |= HW_POWER_MISC_FREQSEL_20000_KHZ << SHIFT_FREQSEL;
> +		break;
> +	case 24000:
> +		val |= HW_POWER_MISC_FREQSEL_24000_KHZ << SHIFT_FREQSEL;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +	if (ret)
> +		return ret;

Just return directly, it'd be a bit easier to follow.  It'd also be
better to print an error message saying we're rejecting the value - that
will make it easier for someone writing a DT to figure out that they
have made a typo or whatever.

> +static int mxs_ldo_set_voltage_sel(struct regulator_dev *reg, unsigned sel)
> +{
> +	struct mxs_ldo *ldo = rdev_get_drvdata(reg);
> +	struct regulator_desc *desc = &ldo->desc;
> +	unsigned long start;
> +	u32 regs;
> +	int uV;
> +	u8 power_source = HW_POWER_UNKNOWN_SOURCE;
> +
> +	uV = regulator_list_voltage_linear(reg, sel);

This would be strange execpt uV doesn't seem to be referenced at all so
it could be removed instead.

> +	if (ldo->get_power_source)
> +		power_source = ldo->get_power_source(reg);
> +
> +	switch (power_source) {
> +	case HW_POWER_LINREG_DCDC_OFF:
> +	case HW_POWER_LINREG_DCDC_READY:
> +	case HW_POWER_EXTERNAL_SOURCE_5V:
> +		usleep_range(1000, 2000);
> +		return 0;
> +	}

I'd expect the switch to be in the if here?

> +
> +	usleep_range(15, 20);
> +	start = jiffies;
> +	while (1) {
> +		if (readl(ldo->status_addr) & BM_POWER_STS_DC_OK)
> +			return 0;
> +
> +		if (time_after(jiffies, start +	msecs_to_jiffies(20)))
> +			break;
> +
> +		schedule();
> +	}

So, this isn't actually quite a busy wait because we do a schedule()
rather than a cpu_relax() but still it could devolve into that - 20ms
seems a long time to burn doing that.  If we're expecting this to finish
very quickly can we do an initial busy wait then fall back to something
with an actual sleep or soemthing?

> +static int mxs_ldo_get_voltage_sel(struct regulator_dev *reg)
> +{
> +	struct mxs_ldo *ldo = rdev_get_drvdata(reg);
> +	struct regulator_desc *desc = &ldo->desc;
> +	int ret, uV;
> +
> +	ret = readl(ldo->base_addr) & desc->vsel_mask;
> +	uV = regulator_list_voltage_linear(reg, ret);
> +
> +	return ret;
> +}

Again with the uV lookup...

> +static int mxs_ldo_is_enabled(struct regulator_dev *reg)
> +{
> +	struct mxs_ldo *ldo = rdev_get_drvdata(reg);
> +	u8 power_source = HW_POWER_UNKNOWN_SOURCE;
> +
> +	if (ldo->get_power_source)
> +		power_source = ldo->get_power_source(reg);
> +
> +	switch (power_source) {
> +	case HW_POWER_LINREG_DCDC_OFF:
> +	case HW_POWER_LINREG_DCDC_READY:
> +	case HW_POWER_DCDC_LINREG_ON:
> +		return 1;
> +	}
> +
> +	return 0;
> +}

Some comments explaining what the logic here is might be helpful, it's
all a bit surprising.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150322/ff18cbbb/attachment.sig>

Re: [PATCH 4/7] regulator: add mxs regulator driver

[Stefan Wahren]

Hi Mark,

>
> > + if (ldo->get_power_source)
> > + power_source = ldo->get_power_source(reg);
> > +
> > + switch (power_source) {
> > + case HW_POWER_LINREG_DCDC_OFF:
> > + case HW_POWER_LINREG_DCDC_READY:
> > + case HW_POWER_EXTERNAL_SOURCE_5V:
> > + usleep_range(1000, 2000);
> > + return 0;
> > + }
>
> I'd expect the switch to be in the if here?

yes, that's possible too.

>
> > +
> > + usleep_range(15, 20);
> > + start = jiffies;
> > + while (1) {
> > + if (readl(ldo->status_addr) & BM_POWER_STS_DC_OK)
> > + return 0;
> > +
> > + if (time_after(jiffies, start + msecs_to_jiffies(20)))
> > + break;
> > +
> > + schedule();
> > + }
>
> So, this isn't actually quite a busy wait because we do a schedule()
> rather than a cpu_relax() but still it could devolve into that - 20ms
> seems a long time to burn doing that. If we're expecting this to finish
> very quickly can we do an initial busy wait then fall back to something
> with an actual sleep or soemthing?

We will need to keep the initial usleep_range(). So do you think of the
following:

1. usleep_range
2. busy_wait for BM_POWER_STS_DC_OK
3. msleep (for the timeout case)

Stefan

[PATCH 4/7] regulator: add mxs regulator driver

[Stefan Wahren]

Hi Mark,

>
> > + if (ldo->get_power_source)
> > + power_source = ldo->get_power_source(reg);
> > +
> > + switch (power_source) {
> > + case HW_POWER_LINREG_DCDC_OFF:
> > + case HW_POWER_LINREG_DCDC_READY:
> > + case HW_POWER_EXTERNAL_SOURCE_5V:
> > + usleep_range(1000, 2000);
> > + return 0;
> > + }
>
> I'd expect the switch to be in the if here?

yes, that's possible too.

>
> > +
> > + usleep_range(15, 20);
> > + start = jiffies;
> > + while (1) {
> > + if (readl(ldo->status_addr) & BM_POWER_STS_DC_OK)
> > + return 0;
> > +
> > + if (time_after(jiffies, start + msecs_to_jiffies(20)))
> > + break;
> > +
> > + schedule();
> > + }
>
> So, this isn't actually quite a busy wait because we do a schedule()
> rather than a cpu_relax() but still it could devolve into that - 20ms
> seems a long time to burn doing that. If we're expecting this to finish
> very quickly can we do an initial busy wait then fall back to something
> with an actual sleep or soemthing?

We will need to keep the initial usleep_range(). So do you think of the
following:

1. usleep_range
2. busy_wait for BM_POWER_STS_DC_OK
3. msleep (for the timeout case)

Stefan

Re: [PATCH 4/7] regulator: add mxs regulator driver

[Mark Brown]

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

On Mon, Mar 23, 2015 at 06:45:41PM +0100, Stefan Wahren wrote:

> > So, this isn't actually quite a busy wait because we do a schedule()
> > rather than a cpu_relax() but still it could devolve into that - 20ms
> > seems a long time to burn doing that. If we're expecting this to finish
> > very quickly can we do an initial busy wait then fall back to something
> > with an actual sleep or soemthing?

> We will need to keep the initial usleep_range(). So do you think of the
> following:

> 1. usleep_range
> 2. busy_wait for BM_POWER_STS_DC_OK
> 3. msleep (for the timeout case)

That's what I'm suggesting - do the initial busy wait for a reasonably
short time period and then fall back to something that sleeps.

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

[PATCH 4/7] regulator: add mxs regulator driver

[Mark Brown]

On Mon, Mar 23, 2015 at 06:45:41PM +0100, Stefan Wahren wrote:

> > So, this isn't actually quite a busy wait because we do a schedule()
> > rather than a cpu_relax() but still it could devolve into that - 20ms
> > seems a long time to burn doing that. If we're expecting this to finish
> > very quickly can we do an initial busy wait then fall back to something
> > with an actual sleep or soemthing?

> We will need to keep the initial usleep_range(). So do you think of the
> following:

> 1. usleep_range
> 2. busy_wait for BM_POWER_STS_DC_OK
> 3. msleep (for the timeout case)

That's what I'm suggesting - do the initial busy wait for a reasonably
short time period and then fall back to something that sleeps.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150323/d00b8429/attachment.sig>

[PATCH 5/7] 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 |   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;
+				};
+
+				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 {
-- 
1.7.9.5

[PATCH 5/7] 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 |   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 at 80044000 {
+				compatible = "fsl,imx23-power";
+				#address-cells = <1>;
+				#size-cells = <1>;
 				reg = <0x80044000 0x2000>;
-				status = "disabled";
+				interrupts = <3>;
+				ranges;
+
+				dcdc: regulator at 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 at 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 at 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 at 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 at 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 at 80044000 {
+				compatible = "fsl,imx28-power";
+				#address-cells = <1>;
+				#size-cells = <1>;
 				reg = <0x80044000 0x2000>;
-				status = "disabled";
+				interrupts = <6>;
+				ranges;
+
+				dcdc: regulator at 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 at 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 at 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 at 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 at 80046000 {
-- 
1.7.9.5

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

[Sascha Hauer]

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

-- 
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 |

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

[Sascha Hauer]

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 at 80044000 {
> +				compatible = "fsl,imx23-power";
> +				#address-cells = <1>;
> +				#size-cells = <1>;
>  				reg = <0x80044000 0x2000>;
> -				status = "disabled";
> +				interrupts = <3>;
> +				ranges;
> +
> +				dcdc: regulator at 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

-- 
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 |

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

[Stefan Wahren]

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 |

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

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

[Stefan Wahren]

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 at 80044000 {
> > + compatible = "fsl,imx23-power";
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > reg = <0x80044000 0x2000>;
> > - status = "disabled";
> > + interrupts = <3>;
> > + ranges;
> > +
> > + dcdc: regulator at 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 |

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

[Mark Brown]

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

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.

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

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

[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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150323/9c3f8374/attachment.sig>

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

[Stefan Wahren]

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-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> 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 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 5/7] ARM: dts: enable regulator support for i.MX23/i.MX28

[Stefan Wahren]

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

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

[Mark Brown]

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

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.

[-- 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

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

[Mark Brown]

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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150324/14a7e477/attachment.sig>

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

[Sascha Hauer]

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


-- 
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 |

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

[Sascha Hauer]

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


-- 
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 |

[PATCH 6/7] ARM: mxs: register cpufreq-dt in pm init

[Stefan Wahren]

This patch adds the registration of cpufreq-dt into pm init to get
cpufreq support for MXS platform.

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

diff --git a/arch/arm/mach-mxs/pm.c b/arch/arm/mach-mxs/pm.c
index 0170e99..23c52e0 100644
--- a/arch/arm/mach-mxs/pm.c
+++ b/arch/arm/mach-mxs/pm.c
@@ -15,6 +15,7 @@
 #include <linux/kernel.h>
 #include <linux/suspend.h>
 #include <linux/io.h>
+#include <linux/platform_device.h>
 #include "pm.h"
 
 static int mxs_suspend_enter(suspend_state_t state)
@@ -35,7 +36,12 @@ static struct platform_suspend_ops mxs_suspend_ops = {
 	.valid = suspend_valid_only_mem,
 };
 
+static struct platform_device mxs_cpufreq_pdev = {
+	.name = "cpufreq-dt",
+};
+
 void __init mxs_pm_init(void)
 {
 	suspend_set_ops(&mxs_suspend_ops);
+	platform_device_register(&mxs_cpufreq_pdev);
 }
-- 
1.7.9.5

[PATCH 6/7] ARM: mxs: register cpufreq-dt in pm init

[Stefan Wahren]

This patch adds the registration of cpufreq-dt into pm init to get
cpufreq support for MXS platform.

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

diff --git a/arch/arm/mach-mxs/pm.c b/arch/arm/mach-mxs/pm.c
index 0170e99..23c52e0 100644
--- a/arch/arm/mach-mxs/pm.c
+++ b/arch/arm/mach-mxs/pm.c
@@ -15,6 +15,7 @@
 #include <linux/kernel.h>
 #include <linux/suspend.h>
 #include <linux/io.h>
+#include <linux/platform_device.h>
 #include "pm.h"
 
 static int mxs_suspend_enter(suspend_state_t state)
@@ -35,7 +36,12 @@ static struct platform_suspend_ops mxs_suspend_ops = {
 	.valid = suspend_valid_only_mem,
 };
 
+static struct platform_device mxs_cpufreq_pdev = {
+	.name = "cpufreq-dt",
+};
+
 void __init mxs_pm_init(void)
 {
 	suspend_set_ops(&mxs_suspend_ops);
+	platform_device_register(&mxs_cpufreq_pdev);
 }
-- 
1.7.9.5

[PATCH 7/7] ARM: dts: add OPPs for i.MX23/i.MX28

[Stefan Wahren]

This patch adds the OPPs for i.MX23/i.MX28 which are necessary
for cpufreq support.

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

diff --git a/arch/arm/boot/dts/imx23.dtsi b/arch/arm/boot/dts/imx23.dtsi
index be0aee8..885c79c 100644
--- a/arch/arm/boot/dts/imx23.dtsi
+++ b/arch/arm/boot/dts/imx23.dtsi
@@ -27,12 +27,23 @@
 	};
 
 	cpus {
-		#address-cells = <0>;
+		#address-cells = <1>;
 		#size-cells = <0>;
 
-		cpu {
+		cpu@0 {
 			compatible = "arm,arm926ej-s";
 			device_type = "cpu";
+			reg = <0x0>;
+			operating-points = <
+				/* kHz	uV */
+				261819  1350000
+				360000  1350000
+				392728  1450000
+				454737  1550000
+			>;
+			clocks = <&clks 2>;
+			clock-latency = <61036>; /* two CLK32 periods */
+			cpu-supply = <&reg_vddd>;
 		};
 	};
 
diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index 98c1be6..21c1921 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -38,12 +38,23 @@
 	};
 
 	cpus {
-		#address-cells = <0>;
+		#address-cells = <1>;
 		#size-cells = <0>;
 
-		cpu {
+		cpu@0 {
 			compatible = "arm,arm926ej-s";
 			device_type = "cpu";
+			reg = <0x0>;
+			operating-points = <
+				/* kHz	uV */
+				261819  1350000
+				360000  1350000
+				392728  1450000
+				454737  1550000
+			>;
+			clocks = <&clks 4>;
+			clock-latency = <61036>; /* two CLK32 periods */
+			cpu-supply = <&reg_vddd>;
 		};
 	};
 
-- 
1.7.9.5

[PATCH 7/7] ARM: dts: add OPPs for i.MX23/i.MX28

[Stefan Wahren]

This patch adds the OPPs for i.MX23/i.MX28 which are necessary
for cpufreq support.

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

diff --git a/arch/arm/boot/dts/imx23.dtsi b/arch/arm/boot/dts/imx23.dtsi
index be0aee8..885c79c 100644
--- a/arch/arm/boot/dts/imx23.dtsi
+++ b/arch/arm/boot/dts/imx23.dtsi
@@ -27,12 +27,23 @@
 	};
 
 	cpus {
-		#address-cells = <0>;
+		#address-cells = <1>;
 		#size-cells = <0>;
 
-		cpu {
+		cpu at 0 {
 			compatible = "arm,arm926ej-s";
 			device_type = "cpu";
+			reg = <0x0>;
+			operating-points = <
+				/* kHz	uV */
+				261819  1350000
+				360000  1350000
+				392728  1450000
+				454737  1550000
+			>;
+			clocks = <&clks 2>;
+			clock-latency = <61036>; /* two CLK32 periods */
+			cpu-supply = <&reg_vddd>;
 		};
 	};
 
diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index 98c1be6..21c1921 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -38,12 +38,23 @@
 	};
 
 	cpus {
-		#address-cells = <0>;
+		#address-cells = <1>;
 		#size-cells = <0>;
 
-		cpu {
+		cpu at 0 {
 			compatible = "arm,arm926ej-s";
 			device_type = "cpu";
+			reg = <0x0>;
+			operating-points = <
+				/* kHz	uV */
+				261819  1350000
+				360000  1350000
+				392728  1450000
+				454737  1550000
+			>;
+			clocks = <&clks 4>;
+			clock-latency = <61036>; /* two CLK32 periods */
+			cpu-supply = <&reg_vddd>;
 		};
 	};
 
-- 
1.7.9.5

Re: [PATCH 7/7] ARM: dts: add OPPs for i.MX23/i.MX28

[Juergen Borleis]

Stefan Wahren wrote:
> [...]
> diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> index 98c1be6..21c1921 100644
> --- a/arch/arm/boot/dts/imx28.dtsi
> +++ b/arch/arm/boot/dts/imx28.dtsi
> @@ -38,12 +38,23 @@
>  	};
>
>  	cpus {
> -		#address-cells = <0>;
> +		#address-cells = <1>;
>  		#size-cells = <0>;
>
> -		cpu {
> +		cpu@0 {
>  			compatible = "arm,arm926ej-s";
>  			device_type = "cpu";
> +			reg = <0x0>;
> +			operating-points = <
> +				/* kHz	uV */
> +				261819  1350000
> +				360000  1350000
> +				392728  1450000
> +				454737  1550000
> +			>;
> +			clocks = <&clks 4>;
> +			clock-latency = <61036>; /* two CLK32 periods */
> +			cpu-supply = <&reg_vddd>;
>  		};
>  	};

Maybe you should take into account not to reduce VDD below 1.55 V if the SDRAM 
controller runs above 196 MHz. The i.MX28 datasheet[1] lists these 
restrictions. VDD powers the SDRAM controller as well. From the datasheet the 
table "Frequency versus Voltage for EMICLK" shows:
                     
             EMICLK Fmax (MHz)
VDDD (V)      DDR2       mDDR
--------------------------------
 1.550       205.71     205.71
 1.450       196.36     196.36
 1.350       196.36     196.36

jbe

[1]
  i.MX28 Applications
Processors for Consumer
     Products
  Silicon Version 1.2

Document Number: IMX28CEC
   Rev. 3, 07/2012

[PATCH 7/7] ARM: dts: add OPPs for i.MX23/i.MX28

[Juergen Borleis]

Stefan Wahren wrote:
> [...]
> diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> index 98c1be6..21c1921 100644
> --- a/arch/arm/boot/dts/imx28.dtsi
> +++ b/arch/arm/boot/dts/imx28.dtsi
> @@ -38,12 +38,23 @@
>  	};
>
>  	cpus {
> -		#address-cells = <0>;
> +		#address-cells = <1>;
>  		#size-cells = <0>;
>
> -		cpu {
> +		cpu at 0 {
>  			compatible = "arm,arm926ej-s";
>  			device_type = "cpu";
> +			reg = <0x0>;
> +			operating-points = <
> +				/* kHz	uV */
> +				261819  1350000
> +				360000  1350000
> +				392728  1450000
> +				454737  1550000
> +			>;
> +			clocks = <&clks 4>;
> +			clock-latency = <61036>; /* two CLK32 periods */
> +			cpu-supply = <&reg_vddd>;
>  		};
>  	};

Maybe you should take into account not to reduce VDD below 1.55 V if the SDRAM 
controller runs above 196 MHz. The i.MX28 datasheet[1] lists these 
restrictions. VDD powers the SDRAM controller as well. From the datasheet the 
table "Frequency versus Voltage for EMICLK" shows:
                     
             EMICLK Fmax (MHz)
VDDD (V)      DDR2       mDDR
--------------------------------
 1.550       205.71     205.71
 1.450       196.36     196.36
 1.350       196.36     196.36

jbe

[1]
  i.MX28 Applications
Processors for Consumer
     Products
  Silicon Version 1.2

Document Number: IMX28CEC
   Rev. 3, 07/2012

Re: [PATCH 7/7] ARM: dts: add OPPs for i.MX23/i.MX28

[Stefan Wahren]

Hi Juergen,

> Juergen Borleis <juergen-vozXLyro3r7AVMDmWPUVSw@public.gmane.org> hat am 24. März 2015 um 21:45
> geschrieben:
>
>
> Stefan Wahren wrote:
> > [...]
> 
> Maybe you should take into account not to reduce VDD below 1.55 V if the SDRAM
> controller runs above 196 MHz. The i.MX28 datasheet[1] lists these
> restrictions. VDD powers the SDRAM controller as well. From the datasheet the
> table "Frequency versus Voltage for EMICLK" shows:
>
> EMICLK Fmax (MHz)
> VDDD (V) DDR2 mDDR
> --------------------------------
> 1.550 205.71 205.71
> 1.450 196.36 196.36
> 1.350 196.36 196.36
>
> jbe
>
> [1]
> i.MX28 Applications
> Processors for Consumer
> Products
> Silicon Version 1.2
>
> Document Number: IMX28CEC
> Rev. 3, 07/2012

thanks for pointing out. I will take care of it.

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 7/7] ARM: dts: add OPPs for i.MX23/i.MX28

[Stefan Wahren]

Hi Juergen,

> Juergen Borleis <juergen@kreuzholzen.de> hat am 24. M?rz 2015 um 21:45
> geschrieben:
>
>
> Stefan Wahren wrote:
> > [...]
> 
> Maybe you should take into account not to reduce VDD below 1.55 V if the SDRAM
> controller runs above 196 MHz. The i.MX28 datasheet[1] lists these
> restrictions. VDD powers the SDRAM controller as well. From the datasheet the
> table "Frequency versus Voltage for EMICLK" shows:
>
> EMICLK Fmax (MHz)
> VDDD (V) DDR2 mDDR
> --------------------------------
> 1.550 205.71 205.71
> 1.450 196.36 196.36
> 1.350 196.36 196.36
>
> jbe
>
> [1]
> i.MX28 Applications
> Processors for Consumer
> Products
> Silicon Version 1.2
>
> Document Number: IMX28CEC
> Rev. 3, 07/2012

thanks for pointing out. I will take care of it.

Stefan

Re: [PATCH 7/7] ARM: dts: add OPPs for i.MX23/i.MX28

[Stefan Wahren]

Hi,

> Juergen Borleis <juergen@kreuzholzen.de> hat am 24. März 2015 um 21:45
> geschrieben:
>
>
> Stefan Wahren wrote:
> > [...]
> > diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> > index 98c1be6..21c1921 100644
> > --- a/arch/arm/boot/dts/imx28.dtsi
> > +++ b/arch/arm/boot/dts/imx28.dtsi
> > @@ -38,12 +38,23 @@
> > };
> >
> > cpus {
> > - #address-cells = <0>;
> > + #address-cells = <1>;
> > #size-cells = <0>;
> >
> > - cpu {
> > + cpu@0 {
> > compatible = "arm,arm926ej-s";
> > device_type = "cpu";
> > + reg = <0x0>;
> > + operating-points = <
> > + /* kHz uV */
> > + 261819 1350000
> > + 360000 1350000
> > + 392728 1450000
> > + 454737 1550000
> > + >;
> > + clocks = <&clks 4>;
> > + clock-latency = <61036>; /* two CLK32 periods */
> > + cpu-supply = <&reg_vddd>;
> > };
> > };
>
> Maybe you should take into account not to reduce VDD below 1.55 V if the SDRAM
> controller runs above 196 MHz. The i.MX28 datasheet[1] lists these
> restrictions. VDD powers the SDRAM controller as well. From the datasheet the
> table "Frequency versus Voltage for EMICLK" shows:
>
> EMICLK Fmax (MHz)
> VDDD (V) DDR2 mDDR
> --------------------------------
> 1.550 205.71 205.71
> 1.450 196.36 196.36
> 1.350 196.36 196.36
>
> jbe
>
> [1]
> i.MX28 Applications
> Processors for Consumer
> Products
> Silicon Version 1.2
>
> Document Number: IMX28CEC
> Rev. 3, 07/2012

the only chance that i see to meet this constraint without introducing a new
cpufreq driver is to change clock ref_cpu
and ref_emi.

Do you think this solution of a virtual clock [1] can be applied here, too?

Best regards
Stefan

[1] -
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/290822.html

Re: [PATCH 7/7] ARM: dts: add OPPs for i.MX23/i.MX28

[Juergen Borleis]

Hi Stefan,

Stefan Wahren wrote:
> > Juergen Borleis <juergen@kreuzholzen.de> hat am 24. März 2015 um 21:45
> > geschrieben:
> >
> > Stefan Wahren wrote:
> > > [...]
> > > diff --git a/arch/arm/boot/dts/imx28.dtsi
> > > b/arch/arm/boot/dts/imx28.dtsi index 98c1be6..21c1921 100644
> > > --- a/arch/arm/boot/dts/imx28.dtsi
> > > +++ b/arch/arm/boot/dts/imx28.dtsi
> > > @@ -38,12 +38,23 @@
> > > };
> > >
> > > cpus {
> > > - #address-cells = <0>;
> > > + #address-cells = <1>;
> > > #size-cells = <0>;
> > >
> > > - cpu {
> > > + cpu@0 {
> > > compatible = "arm,arm926ej-s";
> > > device_type = "cpu";
> > > + reg = <0x0>;
> > > + operating-points = <
> > > + /* kHz uV */
> > > + 261819 1350000
> > > + 360000 1350000
> > > + 392728 1450000
> > > + 454737 1550000
> > > + >;
> > > + clocks = <&clks 4>;
> > > + clock-latency = <61036>; /* two CLK32 periods */
> > > + cpu-supply = <&reg_vddd>;
> > > };
> > > };
> >
> > Maybe you should take into account not to reduce VDD below 1.55 V if the
> > SDRAM controller runs above 196 MHz. The i.MX28 datasheet[1] lists these
> > restrictions. VDD powers the SDRAM controller as well. From the datasheet
> > the table "Frequency versus Voltage for EMICLK" shows:
> >
> > EMICLK Fmax (MHz)
> > VDDD (V) DDR2 mDDR
> > --------------------------------
> > 1.550 205.71 205.71
> > 1.450 196.36 196.36
> > 1.350 196.36 196.36
> >
> > jbe
> >
> > [1]
> > i.MX28 Applications
> > Processors for Consumer
> > Products
> > Silicon Version 1.2
> >
> > Document Number: IMX28CEC
> > Rev. 3, 07/2012
>
> the only chance that i see to meet this constraint without introducing a
> new cpufreq driver is to change clock ref_cpu
> and ref_emi.
>
> Do you think this solution of a virtual clock [1] can be applied here, too?

This would imply we can change the SDRAM timing at run-time on the fly. IMHO 
that is not possible (at least not easy). I think when the driver detects the 
SDRAM controller runs above 196 MHz the clock for the CPU core can still be 
changed, but VDD must be kept at 1.55 V.

Juergen

[PATCH 7/7] ARM: dts: add OPPs for i.MX23/i.MX28

[Juergen Borleis]

Hi Stefan,

Stefan Wahren wrote:
> > Juergen Borleis <juergen@kreuzholzen.de> hat am 24. M?rz 2015 um 21:45
> > geschrieben:
> >
> > Stefan Wahren wrote:
> > > [...]
> > > diff --git a/arch/arm/boot/dts/imx28.dtsi
> > > b/arch/arm/boot/dts/imx28.dtsi index 98c1be6..21c1921 100644
> > > --- a/arch/arm/boot/dts/imx28.dtsi
> > > +++ b/arch/arm/boot/dts/imx28.dtsi
> > > @@ -38,12 +38,23 @@
> > > };
> > >
> > > cpus {
> > > - #address-cells = <0>;
> > > + #address-cells = <1>;
> > > #size-cells = <0>;
> > >
> > > - cpu {
> > > + cpu at 0 {
> > > compatible = "arm,arm926ej-s";
> > > device_type = "cpu";
> > > + reg = <0x0>;
> > > + operating-points = <
> > > + /* kHz uV */
> > > + 261819 1350000
> > > + 360000 1350000
> > > + 392728 1450000
> > > + 454737 1550000
> > > + >;
> > > + clocks = <&clks 4>;
> > > + clock-latency = <61036>; /* two CLK32 periods */
> > > + cpu-supply = <&reg_vddd>;
> > > };
> > > };
> >
> > Maybe you should take into account not to reduce VDD below 1.55 V if the
> > SDRAM controller runs above 196 MHz. The i.MX28 datasheet[1] lists these
> > restrictions. VDD powers the SDRAM controller as well. From the datasheet
> > the table "Frequency versus Voltage for EMICLK" shows:
> >
> > EMICLK Fmax (MHz)
> > VDDD (V) DDR2 mDDR
> > --------------------------------
> > 1.550 205.71 205.71
> > 1.450 196.36 196.36
> > 1.350 196.36 196.36
> >
> > jbe
> >
> > [1]
> > i.MX28 Applications
> > Processors for Consumer
> > Products
> > Silicon Version 1.2
> >
> > Document Number: IMX28CEC
> > Rev. 3, 07/2012
>
> the only chance that i see to meet this constraint without introducing a
> new cpufreq driver is to change clock ref_cpu
> and ref_emi.
>
> Do you think this solution of a virtual clock [1] can be applied here, too?

This would imply we can change the SDRAM timing at run-time on the fly. IMHO 
that is not possible (at least not easy). I think when the driver detects the 
SDRAM controller runs above 196 MHz the clock for the CPU core can still be 
changed, but VDD must be kept at 1.55 V.

Juergen

Re: [PATCH 7/7] ARM: dts: add OPPs for i.MX23/i.MX28

[Stefan Wahren]

Hi Juergen,

> Juergen Borleis <juergen@kreuzholzen.de> hat am 29. März 2015 um 16:40
> geschrieben:
>
>
> Hi Stefan,
>
> Stefan Wahren wrote:
> > > Juergen Borleis <juergen@kreuzholzen.de> hat am 24. März 2015 um 21:45
> > > geschrieben:
> > >
> > > Stefan Wahren wrote:
> > > > [...]
> > > > diff --git a/arch/arm/boot/dts/imx28.dtsi
> > > > b/arch/arm/boot/dts/imx28.dtsi index 98c1be6..21c1921 100644
> > > > --- a/arch/arm/boot/dts/imx28.dtsi
> > > > +++ b/arch/arm/boot/dts/imx28.dtsi
> > > > @@ -38,12 +38,23 @@
> > > > };
> > > >
> > > > cpus {
> > > > - #address-cells = <0>;
> > > > + #address-cells = <1>;
> > > > #size-cells = <0>;
> > > >
> > > > - cpu {
> > > > + cpu@0 {
> > > > compatible = "arm,arm926ej-s";
> > > > device_type = "cpu";
> > > > + reg = <0x0>;
> > > > + operating-points = <
> > > > + /* kHz uV */
> > > > + 261819 1350000
> > > > + 360000 1350000
> > > > + 392728 1450000
> > > > + 454737 1550000
> > > > + >;
> > > > + clocks = <&clks 4>;
> > > > + clock-latency = <61036>; /* two CLK32 periods */
> > > > + cpu-supply = <&reg_vddd>;
> > > > };
> > > > };
> > >
> > > Maybe you should take into account not to reduce VDD below 1.55 V if the
> > > SDRAM controller runs above 196 MHz. The i.MX28 datasheet[1] lists these
> > > restrictions. VDD powers the SDRAM controller as well. From the datasheet
> > > the table "Frequency versus Voltage for EMICLK" shows:
> > >
> > > EMICLK Fmax (MHz)
> > > VDDD (V) DDR2 mDDR
> > > --------------------------------
> > > 1.550 205.71 205.71
> > > 1.450 196.36 196.36
> > > 1.350 196.36 196.36
> > >
> > > jbe
> > >
> > > [1]
> > > i.MX28 Applications
> > > Processors for Consumer
> > > Products
> > > Silicon Version 1.2
> > >
> > > Document Number: IMX28CEC
> > > Rev. 3, 07/2012
> >
> > the only chance that i see to meet this constraint without introducing a
> > new cpufreq driver is to change clock ref_cpu
> > and ref_emi.
> >
> > Do you think this solution of a virtual clock [1] can be applied here, too?
>
> This would imply we can change the SDRAM timing at run-time on the fly. IMHO
> that is not possible (at least not easy).

it's possible since the cpufreq driver in the FSL can do it. But the code isn't
nice and contains some workarounds.

> I think when the driver detects the SDRAM controller runs above 196 MHz the
> clock for the CPU core can still be
> changed, but VDD must be kept at 1.55 V.

The cpufreq-dt doesn't know anything about SDRAM controller and i think it isn't
necessary if we use 1.55 V for all
operating points.

>
> Juergen

Stefan

[PATCH 7/7] ARM: dts: add OPPs for i.MX23/i.MX28

[Stefan Wahren]

Hi Juergen,

> Juergen Borleis <juergen@kreuzholzen.de> hat am 29. M?rz 2015 um 16:40
> geschrieben:
>
>
> Hi Stefan,
>
> Stefan Wahren wrote:
> > > Juergen Borleis <juergen@kreuzholzen.de> hat am 24. M?rz 2015 um 21:45
> > > geschrieben:
> > >
> > > Stefan Wahren wrote:
> > > > [...]
> > > > diff --git a/arch/arm/boot/dts/imx28.dtsi
> > > > b/arch/arm/boot/dts/imx28.dtsi index 98c1be6..21c1921 100644
> > > > --- a/arch/arm/boot/dts/imx28.dtsi
> > > > +++ b/arch/arm/boot/dts/imx28.dtsi
> > > > @@ -38,12 +38,23 @@
> > > > };
> > > >
> > > > cpus {
> > > > - #address-cells = <0>;
> > > > + #address-cells = <1>;
> > > > #size-cells = <0>;
> > > >
> > > > - cpu {
> > > > + cpu at 0 {
> > > > compatible = "arm,arm926ej-s";
> > > > device_type = "cpu";
> > > > + reg = <0x0>;
> > > > + operating-points = <
> > > > + /* kHz uV */
> > > > + 261819 1350000
> > > > + 360000 1350000
> > > > + 392728 1450000
> > > > + 454737 1550000
> > > > + >;
> > > > + clocks = <&clks 4>;
> > > > + clock-latency = <61036>; /* two CLK32 periods */
> > > > + cpu-supply = <&reg_vddd>;
> > > > };
> > > > };
> > >
> > > Maybe you should take into account not to reduce VDD below 1.55 V if the
> > > SDRAM controller runs above 196 MHz. The i.MX28 datasheet[1] lists these
> > > restrictions. VDD powers the SDRAM controller as well. From the datasheet
> > > the table "Frequency versus Voltage for EMICLK" shows:
> > >
> > > EMICLK Fmax (MHz)
> > > VDDD (V) DDR2 mDDR
> > > --------------------------------
> > > 1.550 205.71 205.71
> > > 1.450 196.36 196.36
> > > 1.350 196.36 196.36
> > >
> > > jbe
> > >
> > > [1]
> > > i.MX28 Applications
> > > Processors for Consumer
> > > Products
> > > Silicon Version 1.2
> > >
> > > Document Number: IMX28CEC
> > > Rev. 3, 07/2012
> >
> > the only chance that i see to meet this constraint without introducing a
> > new cpufreq driver is to change clock ref_cpu
> > and ref_emi.
> >
> > Do you think this solution of a virtual clock [1] can be applied here, too?
>
> This would imply we can change the SDRAM timing at run-time on the fly. IMHO
> that is not possible (at least not easy).

it's possible since the cpufreq driver in the FSL can do it. But the code isn't
nice and contains some workarounds.

> I think when the driver detects the SDRAM controller runs above 196 MHz the
> clock for the CPU core can still be
> changed, but VDD must be kept at 1.55 V.

The cpufreq-dt doesn't know anything about SDRAM controller and i think it isn't
necessary if we use 1.55 V for all
operating points.

>
> Juergen

Stefan