[PATCH RFC 0/3] power: mxs_power: add driver for mxs power subsystem

[PATCH RFC 0/3] power: mxs_power: add driver for mxs power subsystem

[Stefan Wahren]

This patch series adds support for Freescale i.MX23, i.xM28 power subsystem.

The idea for this driver was born in the discussion with Mark Rutland
about the mxs-regulator driver [1].

The main use is to trigger driver probing of the underlying DT child nodes
like on-chip regulators. But it's also the first step for cpufreq-dt support.

[1] - https://lkml.org/lkml/2014/9/29/283

Stefan Wahren (3):
  DT: add binding for mxs power subsystem
  power: mxs_power: add driver for mxs power subsystem
  ARM: dts: enable power subsystem for i.MX28

 .../devicetree/bindings/power/mxs_power.txt        |   23 ++
 arch/arm/boot/dts/imx28.dtsi                       |    6 +-
 drivers/power/Kconfig                              |    8 +
 drivers/power/Makefile                             |    1 +
 drivers/power/mxs_power.c                          |  226 ++++++++++++++++++++
 5 files changed, 263 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/power/mxs_power.txt
 create mode 100644 drivers/power/mxs_power.c

--
1.7.9.5

[PATCH RFC 0/3] power: mxs_power: add driver for mxs power subsystem

[Stefan Wahren]

This patch series adds support for Freescale i.MX23, i.xM28 power subsystem.

The idea for this driver was born in the discussion with Mark Rutland
about the mxs-regulator driver [1].

The main use is to trigger driver probing of the underlying DT child nodes
like on-chip regulators. But it's also the first step for cpufreq-dt support.

[1] - https://lkml.org/lkml/2014/9/29/283

Stefan Wahren (3):
  DT: add binding for mxs power subsystem
  power: mxs_power: add driver for mxs power subsystem
  ARM: dts: enable power subsystem for i.MX28

 .../devicetree/bindings/power/mxs_power.txt        |   23 ++
 arch/arm/boot/dts/imx28.dtsi                       |    6 +-
 drivers/power/Kconfig                              |    8 +
 drivers/power/Makefile                             |    1 +
 drivers/power/mxs_power.c                          |  226 ++++++++++++++++++++
 5 files changed, 263 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/power/mxs_power.txt
 create mode 100644 drivers/power/mxs_power.c

--
1.7.9.5

[PATCH RFC 1/3] 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..0817841
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/mxs_power.txt
@@ -0,0 +1,23 @@
+Power subsystem for Freescale MX23/MX28
+
+Required properties:
+- compatible:
+  - "fsl,imx23-power" for MX23
+  - "fsl,imx28-power" for 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 imx28:
+
+	power: power@80044000 {
+				compatible = "fsl,imx28-power";
+				#address-cells = <1>;
+				#size-cells = <1>;
+				reg = <0x80044000 0x2000>;
+				interrupts = <6>;
+				ranges;
+	}
--
1.7.9.5

[PATCH RFC 1/3] 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..0817841
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/mxs_power.txt
@@ -0,0 +1,23 @@
+Power subsystem for Freescale MX23/MX28
+
+Required properties:
+- compatible:
+  - "fsl,imx23-power" for MX23
+  - "fsl,imx28-power" for 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 imx28:
+
+	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 RFC 2/3] 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. Additionally the drivers supports
the configuration of the DC-DC clock frequency to avoid possible
interferences.

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

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 0108c2a..8a3eaa4 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 dfa8942..91c0a62 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..6b68064
--- /dev/null
+++ b/drivers/power/mxs_power.c
@@ -0,0 +1,226 @@
+/*
+ * 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 BM_POWER_CTRL_POLARITY_VBUSVALID	(1 << 5)
+#define BM_POWER_CTRL_VBUSVALID_IRQ		(1 << 4)
+#define BM_POWER_CTRL_ENIRQ_VBUS_VALID		(1 << 3)
+
+#define HW_POWER_5VCTRL_OFFSET	0x10
+#define HW_POWER_MISC_OFFSET	0x90
+
+#define BM_POWER_5VCTRL_VBUSVALID_THRESH	(7 << 8)
+#define BM_POWER_5VCTRL_PWDN_5VBRNOUT		(1 << 7)
+#define BM_POWER_5VCTRL_VBUSVALID_5VDETECT	(1 << 4)
+
+#define HW_POWER_5VCTRL_VBUSVALID_THRESH_4_40V	(5 << 8)
+
+#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		(1 << 0)
+
+static int dcdc_pll;
+module_param(dcdc_pll, int, 0);
+MODULE_PARM_DESC(dcdc_pll,
+		 "DC-DC PLL frequency (kHz). Use 19200, 20000 or 24000");
+
+struct mxs_power_data {
+	void __iomem *base_addr;
+	struct power_supply dc;
+};
+
+int get_dcdc_clk_freq(struct mxs_power_data *pdata)
+{
+	void __iomem *base = pdata->base_addr;
+	int ret = -EINVAL;
+	u32 val;
+
+	val = readl(base + HW_POWER_MISC_OFFSET);
+
+	/* 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_power_data *pdata, int kHz)
+{
+	void __iomem *misc = pdata->base_addr + HW_POWER_MISC_OFFSET;
+	u32 val;
+	int ret = 0;
+
+	val = readl(misc);
+
+	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, misc);
+
+	/* then set PLL as clk for DC-DC converter */
+	writel(val | HW_POWER_MISC_SEL_PLLCLK, misc);
+
+	return 0;
+}
+
+static enum power_supply_property mxs_power_dc_props[] = {
+	POWER_SUPPLY_PROP_ONLINE,
+};
+
+static int mxs_power_dc_get_property(struct power_supply *psy,
+				    enum power_supply_property psp,
+				    union power_supply_propval *val)
+{
+	struct mxs_power_data *data = container_of(psy,
+						   struct mxs_power_data, dc);
+	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 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;
+	int ret;
+	int dcdc_clk_freq;
+
+	if (!np) {
+		dev_err(dev, "missing device tree\n");
+		return -EINVAL;
+	}
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (data == NULL)
+		return -ENOMEM;
+
+	data->dc.properties = mxs_power_dc_props;
+	data->dc.num_properties = ARRAY_SIZE(mxs_power_dc_props);
+	data->dc.get_property = mxs_power_dc_get_property;
+	data->dc.name = "dc";
+	data->dc.type = POWER_SUPPLY_TYPE_MAINS;
+
+	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);
+
+	ret = power_supply_register(dev, &data->dc);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, data);
+
+	if (dcdc_pll)
+		set_dcdc_clk_freq(data, dcdc_pll);
+
+	dcdc_clk_freq = get_dcdc_clk_freq(data);
+
+	dev_info(dev, "DCDC clock freq: %d kHz\n", dcdc_clk_freq);
+
+	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);
+
+	of_platform_depopulate(&pdev->dev);
+	power_supply_unregister(&data->dc);
+
+	return 0;
+}
+
+static struct platform_driver mxs_power_driver = {
+	.driver = {
+		.name	= "mxs_power",
+		.owner  = THIS_MODULE,
+		.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 RFC 2/3] 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. Additionally the drivers supports
the configuration of the DC-DC clock frequency to avoid possible
interferences.

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

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 0108c2a..8a3eaa4 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 dfa8942..91c0a62 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..6b68064
--- /dev/null
+++ b/drivers/power/mxs_power.c
@@ -0,0 +1,226 @@
+/*
+ * 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 BM_POWER_CTRL_POLARITY_VBUSVALID	(1 << 5)
+#define BM_POWER_CTRL_VBUSVALID_IRQ		(1 << 4)
+#define BM_POWER_CTRL_ENIRQ_VBUS_VALID		(1 << 3)
+
+#define HW_POWER_5VCTRL_OFFSET	0x10
+#define HW_POWER_MISC_OFFSET	0x90
+
+#define BM_POWER_5VCTRL_VBUSVALID_THRESH	(7 << 8)
+#define BM_POWER_5VCTRL_PWDN_5VBRNOUT		(1 << 7)
+#define BM_POWER_5VCTRL_VBUSVALID_5VDETECT	(1 << 4)
+
+#define HW_POWER_5VCTRL_VBUSVALID_THRESH_4_40V	(5 << 8)
+
+#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		(1 << 0)
+
+static int dcdc_pll;
+module_param(dcdc_pll, int, 0);
+MODULE_PARM_DESC(dcdc_pll,
+		 "DC-DC PLL frequency (kHz). Use 19200, 20000 or 24000");
+
+struct mxs_power_data {
+	void __iomem *base_addr;
+	struct power_supply dc;
+};
+
+int get_dcdc_clk_freq(struct mxs_power_data *pdata)
+{
+	void __iomem *base = pdata->base_addr;
+	int ret = -EINVAL;
+	u32 val;
+
+	val = readl(base + HW_POWER_MISC_OFFSET);
+
+	/* 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_power_data *pdata, int kHz)
+{
+	void __iomem *misc = pdata->base_addr + HW_POWER_MISC_OFFSET;
+	u32 val;
+	int ret = 0;
+
+	val = readl(misc);
+
+	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, misc);
+
+	/* then set PLL as clk for DC-DC converter */
+	writel(val | HW_POWER_MISC_SEL_PLLCLK, misc);
+
+	return 0;
+}
+
+static enum power_supply_property mxs_power_dc_props[] = {
+	POWER_SUPPLY_PROP_ONLINE,
+};
+
+static int mxs_power_dc_get_property(struct power_supply *psy,
+				    enum power_supply_property psp,
+				    union power_supply_propval *val)
+{
+	struct mxs_power_data *data = container_of(psy,
+						   struct mxs_power_data, dc);
+	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 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;
+	int ret;
+	int dcdc_clk_freq;
+
+	if (!np) {
+		dev_err(dev, "missing device tree\n");
+		return -EINVAL;
+	}
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (data == NULL)
+		return -ENOMEM;
+
+	data->dc.properties = mxs_power_dc_props;
+	data->dc.num_properties = ARRAY_SIZE(mxs_power_dc_props);
+	data->dc.get_property = mxs_power_dc_get_property;
+	data->dc.name = "dc";
+	data->dc.type = POWER_SUPPLY_TYPE_MAINS;
+
+	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);
+
+	ret = power_supply_register(dev, &data->dc);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, data);
+
+	if (dcdc_pll)
+		set_dcdc_clk_freq(data, dcdc_pll);
+
+	dcdc_clk_freq = get_dcdc_clk_freq(data);
+
+	dev_info(dev, "DCDC clock freq: %d kHz\n", dcdc_clk_freq);
+
+	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);
+
+	of_platform_depopulate(&pdev->dev);
+	power_supply_unregister(&data->dc);
+
+	return 0;
+}
+
+static struct platform_driver mxs_power_driver = {
+	.driver = {
+		.name	= "mxs_power",
+		.owner  = THIS_MODULE,
+		.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 RFC 2/3] power: mxs_power: add driver for mxs power subsystem

[Sebastian Reichel]

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

Hi Stefan,

Sorry for the big delay.

On Thu, Nov 20, 2014 at 08:14:36PM +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. Additionally the drivers supports
> the configuration of the DC-DC clock frequency to avoid possible
> interferences.

I would expect PLL to be board specific and part of DT. Why is it
specified as parameter?

Apart from that the driver/patch looks fine to me.

-- Sebastian

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

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

[Sebastian Reichel]

Hi Stefan,

Sorry for the big delay.

On Thu, Nov 20, 2014 at 08:14:36PM +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. Additionally the drivers supports
> the configuration of the DC-DC clock frequency to avoid possible
> interferences.

I would expect PLL to be board specific and part of DT. Why is it
specified as parameter?

Apart from that the driver/patch looks fine to me.

-- 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/20150122/672303ef/attachment.sig>

Re: [PATCH RFC 2/3] power: mxs_power: add driver for mxs power subsystem

[Stefan Wahren]

Hi Sebastian,

[add Marek]

Am 22.01.2015 um 00:01 schrieb Sebastian Reichel:
> Hi Stefan,
>
> Sorry for the big delay.

a late answer is better than no answer :)

>
> On Thu, Nov 20, 2014 at 08:14:36PM +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. Additionally the drivers supports
>> the configuration of the DC-DC clock frequency to avoid possible
>> interferences.
> I would expect PLL to be board specific and part of DT. Why is it
> specified as parameter?

I think the switching frequency of the DC-DC belongs to the
configuration and don't describe how the hardware is connected. But i
don't have a problem to change it into a DT property.

Does a common property name exists for the switching frequency or would
it be vendor specific?

> Apart from that the driver/patch looks fine to me.

In the meantime i made some bugfixes on these patches. Now i think it
would be better to integrate the on-chip regulator driver into the patch
series. That would clarify the function of the power driver.

> -- Sebastian

Best regards
Stefan

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

[Stefan Wahren]

Hi Sebastian,

[add Marek]

Am 22.01.2015 um 00:01 schrieb Sebastian Reichel:
> Hi Stefan,
>
> Sorry for the big delay.

a late answer is better than no answer :)

>
> On Thu, Nov 20, 2014 at 08:14:36PM +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. Additionally the drivers supports
>> the configuration of the DC-DC clock frequency to avoid possible
>> interferences.
> I would expect PLL to be board specific and part of DT. Why is it
> specified as parameter?

I think the switching frequency of the DC-DC belongs to the
configuration and don't describe how the hardware is connected. But i
don't have a problem to change it into a DT property.

Does a common property name exists for the switching frequency or would
it be vendor specific?

> Apart from that the driver/patch looks fine to me.

In the meantime i made some bugfixes on these patches. Now i think it
would be better to integrate the on-chip regulator driver into the patch
series. That would clarify the function of the power driver.

> -- Sebastian

Best regards
Stefan

Re: [PATCH RFC 2/3] power: mxs_power: add driver for mxs power subsystem

[Sébastien SZYMANSKI]

Hello,

On 01/22/2015 09:08 AM, Stefan Wahren wrote:
> 
> In the meantime i made some bugfixes on these patches. Now i think it
> would be better to integrate the on-chip regulator driver into the patch
> series. That would clarify the function of the power driver.
> 

I was about to resend this series with a poweroff driver for the imx28
SoCs. Should I wait this series get merged?

I thought adding a poweroff child node like this:

power: power@80044000 {
	...
	poweroff: poweroff@80044100 {
		compatible = "fsl,imx28-poweroff";
		reg = <0x80044100 0x10>;
	};
	...
};

Here is the driver:

/*
 * MXS SoCs reset code.
 *
 * Copyright (c) 2015 Armadeus Systems.
 *
 * Author: Sébastien Szymanski <sebastien.szymanski@armadeus.com>
 *
 * based on imx-snvs-poweroff.c
 *
 * This program is free software; you can redistribute it and/or modify
 * it under the terms of the GNU General Public License version 2 and
 * only version 2 as published by the Free Software Foundation.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 */

#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/platform_device.h>

#define MXS_SET_ADDR                    0x4

#define POWER_RESET_UNLOCK__KEY         (0x3E77 << 16)
#define POWER_RESET_PWD                 0x1

static void __iomem *power_reset;

static void do_mxs_poweroff(void)
{
        writel(POWER_RESET_UNLOCK__KEY | POWER_RESET_PWD,
               power_reset + MXS_SET_ADDR);
}

static int mxs_poweroff_probe(struct platform_device *pdev)
{
        power_reset = of_iomap(pdev->dev.of_node, 0);
        if (!power_reset) {
                dev_err(&pdev->dev, "failed to get memory\n");
                return -ENODEV;
        }

        pm_power_off = do_mxs_poweroff;

        dev_info(&pdev->dev, "power off function set!\n");

        return 0;
}

static const struct of_device_id of_mxs_poweroff_match[] = {
        { .compatible = "fsl,imx28-poweroff", },
        {},
};
MODULE_DEVICE_TABLE(of, of_mxs_poweroff_match);

static struct platform_driver mxs_poweroff_driver = {
        .probe = mxs_poweroff_probe,
        .driver = {
                .name = "mxs-poweroff",
                .of_match_table = of_match_ptr(of_mxs_poweroff_match),
        },
};

static int __init mxs_poweroff_init(void)
{
        return platform_driver_register(&mxs_poweroff_driver);
}
device_initcall(mxs_poweroff_init);

Best Regards,

-- 
Sébastien SZYMANSKI
Software Engineer
Armadeus Systems - A new vision of the embedded world
sebastien.szymanski@armadeus.com
Tel: +33 (0)9 72 29 41 44
Fax: +33 (0)9 72 28 79 26

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

[Sébastien SZYMANSKI]

Hello,

On 01/22/2015 09:08 AM, Stefan Wahren wrote:
> 
> In the meantime i made some bugfixes on these patches. Now i think it
> would be better to integrate the on-chip regulator driver into the patch
> series. That would clarify the function of the power driver.
> 

I was about to resend this series with a poweroff driver for the imx28
SoCs. Should I wait this series get merged?

I thought adding a poweroff child node like this:

power: power at 80044000 {
	...
	poweroff: poweroff at 80044100 {
		compatible = "fsl,imx28-poweroff";
		reg = <0x80044100 0x10>;
	};
	...
};

Here is the driver:

/*
 * MXS SoCs reset code.
 *
 * Copyright (c) 2015 Armadeus Systems.
 *
 * Author: S?bastien Szymanski <sebastien.szymanski@armadeus.com>
 *
 * based on imx-snvs-poweroff.c
 *
 * This program is free software; you can redistribute it and/or modify
 * it under the terms of the GNU General Public License version 2 and
 * only version 2 as published by the Free Software Foundation.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 */

#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/platform_device.h>

#define MXS_SET_ADDR                    0x4

#define POWER_RESET_UNLOCK__KEY         (0x3E77 << 16)
#define POWER_RESET_PWD                 0x1

static void __iomem *power_reset;

static void do_mxs_poweroff(void)
{
        writel(POWER_RESET_UNLOCK__KEY | POWER_RESET_PWD,
               power_reset + MXS_SET_ADDR);
}

static int mxs_poweroff_probe(struct platform_device *pdev)
{
        power_reset = of_iomap(pdev->dev.of_node, 0);
        if (!power_reset) {
                dev_err(&pdev->dev, "failed to get memory\n");
                return -ENODEV;
        }

        pm_power_off = do_mxs_poweroff;

        dev_info(&pdev->dev, "power off function set!\n");

        return 0;
}

static const struct of_device_id of_mxs_poweroff_match[] = {
        { .compatible = "fsl,imx28-poweroff", },
        {},
};
MODULE_DEVICE_TABLE(of, of_mxs_poweroff_match);

static struct platform_driver mxs_poweroff_driver = {
        .probe = mxs_poweroff_probe,
        .driver = {
                .name = "mxs-poweroff",
                .of_match_table = of_match_ptr(of_mxs_poweroff_match),
        },
};

static int __init mxs_poweroff_init(void)
{
        return platform_driver_register(&mxs_poweroff_driver);
}
device_initcall(mxs_poweroff_init);

Best Regards,

-- 
S?bastien SZYMANSKI
Software Engineer
Armadeus Systems - A new vision of the embedded world
sebastien.szymanski at armadeus.com
Tel: +33 (0)9 72 29 41 44
Fax: +33 (0)9 72 28 79 26

Re: [PATCH RFC 2/3] power: mxs_power: add driver for mxs power subsystem

[Stefan Wahren]

Hello,

Am 22.01.2015 um 10:37 schrieb Sébastien SZYMANSKI:
> Hello,
>
> On 01/22/2015 09:08 AM, Stefan Wahren wrote:
>> In the meantime i made some bugfixes on these patches. Now i think it
>> would be better to integrate the on-chip regulator driver into the patch
>> series. That would clarify the function of the power driver.
>>
> I was about to resend this series with a poweroff driver for the imx28
> SoCs.

sounds great.

> Should I wait this series get merged?

Yes, please. In the meantime you can use my repo at github (based on
pm+acpi-3.19-rc4):

https://github.com/lategoodbye/linux-mxs-power

>
> I thought adding a poweroff child node like this:
>
> power: power@80044000 {
> 	...
> 	poweroff: poweroff@80044100 {
> 		compatible = "fsl,imx28-poweroff";
> 		reg = <0x80044100 0x10>;
> 	};
> 	...
> };

Just a note to your driver it would be nice if it's handles i.MX23 and
i.MX28.

> [...]

Regards Stefan

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

[Stefan Wahren]

Hello,

Am 22.01.2015 um 10:37 schrieb S?bastien SZYMANSKI:
> Hello,
>
> On 01/22/2015 09:08 AM, Stefan Wahren wrote:
>> In the meantime i made some bugfixes on these patches. Now i think it
>> would be better to integrate the on-chip regulator driver into the patch
>> series. That would clarify the function of the power driver.
>>
> I was about to resend this series with a poweroff driver for the imx28
> SoCs.

sounds great.

> Should I wait this series get merged?

Yes, please. In the meantime you can use my repo at github (based on
pm+acpi-3.19-rc4):

https://github.com/lategoodbye/linux-mxs-power

>
> I thought adding a poweroff child node like this:
>
> power: power at 80044000 {
> 	...
> 	poweroff: poweroff at 80044100 {
> 		compatible = "fsl,imx28-poweroff";
> 		reg = <0x80044100 0x10>;
> 	};
> 	...
> };

Just a note to your driver it would be nice if it's handles i.MX23 and
i.MX28.

> [...]

Regards Stefan

Re: [PATCH RFC 2/3] power: mxs_power: add driver for mxs power subsystem

[Sebastian Reichel]

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

Hi,

[add maintainers from regulator framework]

On Thu, Jan 22, 2015 at 09:08:04AM +0100, Stefan Wahren wrote:
> Am 22.01.2015 um 00:01 schrieb Sebastian Reichel:
> > On Thu, Nov 20, 2014 at 08:14:36PM +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. Additionally the drivers supports
> >> the configuration of the DC-DC clock frequency to avoid possible
> >> interferences.
> > I would expect PLL to be board specific and part of DT. Why is it
> > specified as parameter?
> 
> I think the switching frequency of the DC-DC belongs to the
> configuration and don't describe how the hardware is connected. But i
> don't have a problem to change it into a DT property.
> 
> Does a common property name exists for the switching frequency or would
> it be vendor specific?

As far as I know most regulators have fixed switching frequencies,
so there is no common property name so far. I added regulator
framework people, since this property should be regulator specific.

Apart from introducing a custom/common switching-frequency property
the dts could also expose the PLL like this and use the common
clock-frequency property:

power: power@80044000 {
    compatible = "fsl,imx28-power";
    #address-cells = <1>;
    #size-cells = <1>;
    reg = <0x80044000 0x2000>;
    interrupts = <6>;
    ranges;

    powerpll {
        compatible = "fsl,imx28-power-pll"
        #clock-cells = <0>;
        clock-frequency = <12345>;
    }
}

-- Sebastian

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

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

[Sebastian Reichel]

Hi,

[add maintainers from regulator framework]

On Thu, Jan 22, 2015 at 09:08:04AM +0100, Stefan Wahren wrote:
> Am 22.01.2015 um 00:01 schrieb Sebastian Reichel:
> > On Thu, Nov 20, 2014 at 08:14:36PM +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. Additionally the drivers supports
> >> the configuration of the DC-DC clock frequency to avoid possible
> >> interferences.
> > I would expect PLL to be board specific and part of DT. Why is it
> > specified as parameter?
> 
> I think the switching frequency of the DC-DC belongs to the
> configuration and don't describe how the hardware is connected. But i
> don't have a problem to change it into a DT property.
> 
> Does a common property name exists for the switching frequency or would
> it be vendor specific?

As far as I know most regulators have fixed switching frequencies,
so there is no common property name so far. I added regulator
framework people, since this property should be regulator specific.

Apart from introducing a custom/common switching-frequency property
the dts could also expose the PLL like this and use the common
clock-frequency property:

power: power at 80044000 {
    compatible = "fsl,imx28-power";
    #address-cells = <1>;
    #size-cells = <1>;
    reg = <0x80044000 0x2000>;
    interrupts = <6>;
    ranges;

    powerpll {
        compatible = "fsl,imx28-power-pll"
        #clock-cells = <0>;
        clock-frequency = <12345>;
    }
}

-- 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/20150125/5af2c464/attachment.sig>

Re: [PATCH RFC 2/3] power: mxs_power: add driver for mxs power subsystem

[Stefan Wahren]

Hi,

> Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> hat am 25. Januar 2015 um 16:04
> geschrieben:
>
>
> Hi,
>
> [add maintainers from regulator framework]
>
> On Thu, Jan 22, 2015 at 09:08:04AM +0100, Stefan Wahren wrote:
> > Am 22.01.2015 um 00:01 schrieb Sebastian Reichel:
> > > On Thu, Nov 20, 2014 at 08:14:36PM +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. Additionally the drivers supports
> > >> the configuration of the DC-DC clock frequency to avoid possible
> > >> interferences.
> > > I would expect PLL to be board specific and part of DT. Why is it
> > > specified as parameter?
> >
> > I think the switching frequency of the DC-DC belongs to the
> > configuration and don't describe how the hardware is connected. But i
> > don't have a problem to change it into a DT property.
> >
> > Does a common property name exists for the switching frequency or would
> > it be vendor specific?
>
> As far as I know most regulators have fixed switching frequencies,
> so there is no common property name so far. I added regulator
> framework people, since this property should be regulator specific.
>
> Apart from introducing a custom/common switching-frequency property
> the dts could also expose the PLL like this and use the common
> clock-frequency property:
>
> power: power@80044000 {
> compatible = "fsl,imx28-power";
> #address-cells = <1>;
> #size-cells = <1>;
> reg = <0x80044000 0x2000>;
> interrupts = <6>;
> ranges;
>
> powerpll {
> compatible = "fsl,imx28-power-pll"
> #clock-cells = <0>;
> clock-frequency = <12345>;
> }
> }

okay i understand. But doesn't it need a extra driver to set the switching
frequency because of the new compatible string?

At this point i think about moving this feature into the bootloader.

@Fabio, @Marek:

What's your opinion about that?

>
> -- Sebastian

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 RFC 2/3] power: mxs_power: add driver for mxs power subsystem

[Stefan Wahren]

Hi,

> Sebastian Reichel <sre@kernel.org> hat am 25. Januar 2015 um 16:04
> geschrieben:
>
>
> Hi,
>
> [add maintainers from regulator framework]
>
> On Thu, Jan 22, 2015 at 09:08:04AM +0100, Stefan Wahren wrote:
> > Am 22.01.2015 um 00:01 schrieb Sebastian Reichel:
> > > On Thu, Nov 20, 2014 at 08:14:36PM +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. Additionally the drivers supports
> > >> the configuration of the DC-DC clock frequency to avoid possible
> > >> interferences.
> > > I would expect PLL to be board specific and part of DT. Why is it
> > > specified as parameter?
> >
> > I think the switching frequency of the DC-DC belongs to the
> > configuration and don't describe how the hardware is connected. But i
> > don't have a problem to change it into a DT property.
> >
> > Does a common property name exists for the switching frequency or would
> > it be vendor specific?
>
> As far as I know most regulators have fixed switching frequencies,
> so there is no common property name so far. I added regulator
> framework people, since this property should be regulator specific.
>
> Apart from introducing a custom/common switching-frequency property
> the dts could also expose the PLL like this and use the common
> clock-frequency property:
>
> power: power at 80044000 {
> compatible = "fsl,imx28-power";
> #address-cells = <1>;
> #size-cells = <1>;
> reg = <0x80044000 0x2000>;
> interrupts = <6>;
> ranges;
>
> powerpll {
> compatible = "fsl,imx28-power-pll"
> #clock-cells = <0>;
> clock-frequency = <12345>;
> }
> }

okay i understand. But doesn't it need a extra driver to set the switching
frequency because of the new compatible string?

At this point i think about moving this feature into the bootloader.

@Fabio, @Marek:

What's your opinion about that?

>
> -- Sebastian

Stefan

Re: [PATCH RFC 2/3] power: mxs_power: add driver for mxs power subsystem

[Mark Brown]

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

On Mon, Jan 26, 2015 at 08:46:45PM +0100, Stefan Wahren wrote:

> > > Does a common property name exists for the switching frequency or would
> > > it be vendor specific?

> > As far as I know most regulators have fixed switching frequencies,
> > so there is no common property name so far. I added regulator
> > framework people, since this property should be regulator specific.

No, it's pretty common for there to be a couple of options - there's
normally a performance tradeoff (power for regulation accuracy and/or
cost) which can be selected.  IIRC it's selected at design time as it
affect the choice of passives on the board.

> okay i understand. But doesn't it need a extra driver to set the switching
> frequency because of the new compatible string?

I don't understand this bit at all, sorry.

> 
> At this point i think about moving this feature into the bootloader.
> 
> @Fabio, @Marek:
> 
> What's your opinion about that?
> 
> >
> > -- Sebastian
> 
> Stefan
> 

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

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

[Mark Brown]

On Mon, Jan 26, 2015 at 08:46:45PM +0100, Stefan Wahren wrote:

> > > Does a common property name exists for the switching frequency or would
> > > it be vendor specific?

> > As far as I know most regulators have fixed switching frequencies,
> > so there is no common property name so far. I added regulator
> > framework people, since this property should be regulator specific.

No, it's pretty common for there to be a couple of options - there's
normally a performance tradeoff (power for regulation accuracy and/or
cost) which can be selected.  IIRC it's selected at design time as it
affect the choice of passives on the board.

> okay i understand. But doesn't it need a extra driver to set the switching
> frequency because of the new compatible string?

I don't understand this bit at all, sorry.

> 
> At this point i think about moving this feature into the bootloader.
> 
> @Fabio, @Marek:
> 
> What's your opinion about that?
> 
> >
> > -- Sebastian
> 
> Stefan
> 
-------------- 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/20150127/224c59a4/attachment.sig>

Re: [PATCH RFC 2/3] power: mxs_power: add driver for mxs power subsystem

[Stefan Wahren]

Hi Mark,

> Mark Brown <broonie@kernel.org> hat am 27. Januar 2015 um 01:16 geschrieben:
>
>
> On Mon, Jan 26, 2015 at 08:46:45PM +0100, Stefan Wahren wrote:
>
> > > > Does a common property name exists for the switching frequency or would
> > > > it be vendor specific?
>
> > > As far as I know most regulators have fixed switching frequencies,
> > > so there is no common property name so far. I added regulator
> > > framework people, since this property should be regulator specific.
>
> No, it's pretty common for there to be a couple of options - there's
> normally a performance tradeoff (power for regulation accuracy and/or
> cost) which can be selected. IIRC it's selected at design time as it
> affect the choice of passives on the board.
>
> > okay i understand. But doesn't it need a extra driver to set the switching
> > frequency because of the new compatible string?
>
> I don't understand this bit at all, sorry.

Sebastian suggested a new sub-node in the devicetree:

    powerpll {
        compatible = "fsl,imx28-power-pll"
        #clock-cells = <0>;
        clock-frequency = <12345>;
    }

and i think that the new compatible string needs a separate driver to take care
of the switching frequency.

Or is it okay to leave the handling of the switching frequency in the mxs-power
driver?

Btw if we add a new node to set switching frequency, i think it would be better
to describe the dc-dc convertor and not the pll:

    dcdc {
       compatible = "fsl,imx28-dcdc"
       frequency = <12345>;
    }

What do you think?

Best regards
Stefan

>
> >
> > At this point i think about moving this feature into the bootloader.
> >
> > @Fabio, @Marek:
> >
> > What's your opinion about that?
> >
> > >
> > > -- Sebastian
> >
> > Stefan
> >

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

[Stefan Wahren]

Hi Mark,

> Mark Brown <broonie@kernel.org> hat am 27. Januar 2015 um 01:16 geschrieben:
>
>
> On Mon, Jan 26, 2015 at 08:46:45PM +0100, Stefan Wahren wrote:
>
> > > > Does a common property name exists for the switching frequency or would
> > > > it be vendor specific?
>
> > > As far as I know most regulators have fixed switching frequencies,
> > > so there is no common property name so far. I added regulator
> > > framework people, since this property should be regulator specific.
>
> No, it's pretty common for there to be a couple of options - there's
> normally a performance tradeoff (power for regulation accuracy and/or
> cost) which can be selected. IIRC it's selected at design time as it
> affect the choice of passives on the board.
>
> > okay i understand. But doesn't it need a extra driver to set the switching
> > frequency because of the new compatible string?
>
> I don't understand this bit at all, sorry.

Sebastian suggested a new sub-node in the devicetree:

    powerpll {
        compatible = "fsl,imx28-power-pll"
        #clock-cells = <0>;
        clock-frequency = <12345>;
    }

and i think that the new compatible string needs a separate driver to take care
of the switching frequency.

Or is it okay to leave the handling of the switching frequency in the mxs-power
driver?

Btw if we add a new node to set switching frequency, i think it would be better
to describe the dc-dc convertor and not the pll:

    dcdc {
       compatible = "fsl,imx28-dcdc"
       frequency = <12345>;
    }

What do you think?

Best regards
Stefan

>
> >
> > At this point i think about moving this feature into the bootloader.
> >
> > @Fabio, @Marek:
> >
> > What's your opinion about that?
> >
> > >
> > > -- Sebastian
> >
> > Stefan
> >

Re: [PATCH RFC 2/3] power: mxs_power: add driver for mxs power subsystem

[Mark Brown]

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

On Tue, Jan 27, 2015 at 07:35:09PM +0100, Stefan Wahren wrote:
> > Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> hat am 27. Januar 2015 um 01:16 geschrieben:

> > > okay i understand. But doesn't it need a extra driver to set the switching
> > > frequency because of the new compatible string?

> > I don't understand this bit at all, sorry.

> Sebastian suggested a new sub-node in the devicetree:

>     powerpll {
>         compatible = "fsl,imx28-power-pll"
>         #clock-cells = <0>;
>         clock-frequency = <12345>;
>     }

> and i think that the new compatible string needs a separate driver to take care
> of the switching frequency.

I can see that this has been suggested, what I can't understand is why
we would wish to do this but I am missing some context here which may
make it all perfectly clear.

> Or is it okay to leave the handling of the switching frequency in the mxs-power
> driver?

Unless the clock is exposed externally to the regulator either as an
input or an output I'm not sure I see any benefit but I'm possibly
missing something here.  It does sound like there might be a separate
clock IP or something?

> Btw if we add a new node to set switching frequency, i think it would be better
> to describe the dc-dc convertor and not the pll:

>     dcdc {
>        compatible = "fsl,imx28-dcdc"
>        frequency = <12345>;
>     }

> What do you think?

Yes, just adding a new property on the regulator node with all the other
properties seems sensible (we've just gained some helpers which make it
fairly straightforward to do this).

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

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

[Mark Brown]

On Tue, Jan 27, 2015 at 07:35:09PM +0100, Stefan Wahren wrote:
> > Mark Brown <broonie@kernel.org> hat am 27. Januar 2015 um 01:16 geschrieben:

> > > okay i understand. But doesn't it need a extra driver to set the switching
> > > frequency because of the new compatible string?

> > I don't understand this bit at all, sorry.

> Sebastian suggested a new sub-node in the devicetree:

>     powerpll {
>         compatible = "fsl,imx28-power-pll"
>         #clock-cells = <0>;
>         clock-frequency = <12345>;
>     }

> and i think that the new compatible string needs a separate driver to take care
> of the switching frequency.

I can see that this has been suggested, what I can't understand is why
we would wish to do this but I am missing some context here which may
make it all perfectly clear.

> Or is it okay to leave the handling of the switching frequency in the mxs-power
> driver?

Unless the clock is exposed externally to the regulator either as an
input or an output I'm not sure I see any benefit but I'm possibly
missing something here.  It does sound like there might be a separate
clock IP or something?

> Btw if we add a new node to set switching frequency, i think it would be better
> to describe the dc-dc convertor and not the pll:

>     dcdc {
>        compatible = "fsl,imx28-dcdc"
>        frequency = <12345>;
>     }

> What do you think?

Yes, just adding a new property on the regulator node with all the other
properties seems sensible (we've just gained some helpers which make it
fairly straightforward to do this).
-------------- 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/20150127/3e3dfca8/attachment.sig>

Re: [PATCH RFC 2/3] power: mxs_power: add driver for mxs power subsystem

[Stefan Wahren]

Hi Mark,

> Mark Brown <broonie@kernel.org> hat am 27. Januar 2015 um 20:43 geschrieben:
>
>
> On Tue, Jan 27, 2015 at 07:35:09PM +0100, Stefan Wahren wrote:
> > > Mark Brown <broonie@kernel.org> hat am 27. Januar 2015 um 01:16
> > > geschrieben:
>
> > > > okay i understand. But doesn't it need a extra driver to set the
> > > > switching
> > > > frequency because of the new compatible string?
>
> > > I don't understand this bit at all, sorry.
>
> > Sebastian suggested a new sub-node in the devicetree:
>
> > powerpll {
> > compatible = "fsl,imx28-power-pll"
> > #clock-cells = <0>;
> > clock-frequency = <12345>;
> > }
>
> > and i think that the new compatible string needs a separate driver to take
> > care
> > of the switching frequency.
>
> I can see that this has been suggested, what I can't understand is why
> we would wish to do this but I am missing some context here which may
> make it all perfectly clear.

changing the switching frequency is not a common feature for this IC. In most
cases the clock for the DC-DC switching frequency runs at 24 MHz (ref_xtal).
Unfortunatelly this could cause interferences for example in UHF band. So
switching to ref_pll and changing clock divider is a option to avoid such
interferences without changing hardware.

The relevant register DC-DC Miscellaneous Register (HW_POWER_MISC) is described
in chapter 11.12.10 of the reference manual [1].

I decided to implement it as a module parameter, because i see it's a
configuration option.

[1] - http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf

>
> > Or is it okay to leave the handling of the switching frequency in the
> > mxs-power
> > driver?
>
> Unless the clock is exposed externally to the regulator either as an
> input or an output I'm not sure I see any benefit but I'm possibly
> missing something here. It does sound like there might be a separate
> clock IP or something?
>
> > Btw if we add a new node to set switching frequency, i think it would be
> > better
> > to describe the dc-dc convertor and not the pll:
>
> > dcdc {
> > compatible = "fsl,imx28-dcdc"
> > frequency = <12345>;
> > }
>
> > What do you think?
>
> Yes, just adding a new property on the regulator node with all the other
> properties seems sensible (we've just gained some helpers which make it
> fairly straightforward to do this).
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Stefan Wahren]

Hi Mark,

> Mark Brown <broonie@kernel.org> hat am 27. Januar 2015 um 20:43 geschrieben:
>
>
> On Tue, Jan 27, 2015 at 07:35:09PM +0100, Stefan Wahren wrote:
> > > Mark Brown <broonie@kernel.org> hat am 27. Januar 2015 um 01:16
> > > geschrieben:
>
> > > > okay i understand. But doesn't it need a extra driver to set the
> > > > switching
> > > > frequency because of the new compatible string?
>
> > > I don't understand this bit at all, sorry.
>
> > Sebastian suggested a new sub-node in the devicetree:
>
> > powerpll {
> > compatible = "fsl,imx28-power-pll"
> > #clock-cells = <0>;
> > clock-frequency = <12345>;
> > }
>
> > and i think that the new compatible string needs a separate driver to take
> > care
> > of the switching frequency.
>
> I can see that this has been suggested, what I can't understand is why
> we would wish to do this but I am missing some context here which may
> make it all perfectly clear.

changing the switching frequency is not a common feature for this IC. In most
cases the clock for the DC-DC switching frequency runs at 24 MHz (ref_xtal).
Unfortunatelly this could cause interferences for example in UHF band. So
switching to ref_pll and changing clock divider is a option to avoid such
interferences without changing hardware.

The relevant register DC-DC Miscellaneous Register (HW_POWER_MISC) is described
in chapter 11.12.10 of the reference manual [1].

I decided to implement it as a module parameter, because i see it's a
configuration option.

[1] - http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf

>
> > Or is it okay to leave the handling of the switching frequency in the
> > mxs-power
> > driver?
>
> Unless the clock is exposed externally to the regulator either as an
> input or an output I'm not sure I see any benefit but I'm possibly
> missing something here. It does sound like there might be a separate
> clock IP or something?
>
> > Btw if we add a new node to set switching frequency, i think it would be
> > better
> > to describe the dc-dc convertor and not the pll:
>
> > dcdc {
> > compatible = "fsl,imx28-dcdc"
> > frequency = <12345>;
> > }
>
> > What do you think?
>
> Yes, just adding a new property on the regulator node with all the other
> properties seems sensible (we've just gained some helpers which make it
> fairly straightforward to do this).
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH RFC 2/3] power: mxs_power: add driver for mxs power subsystem

[Sebastian Reichel]

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

Hi,

On Wed, Jan 28, 2015 at 11:22:05PM +0100, Stefan Wahren wrote:
> > Mark Brown <broonie@kernel.org> hat am 27. Januar 2015 um 20:43 geschrieben:
> > On Tue, Jan 27, 2015 at 07:35:09PM +0100, Stefan Wahren wrote:
> > > > Mark Brown <broonie@kernel.org> hat am 27. Januar 2015 um 01:16
> > > > geschrieben:
> >
> > > > > okay i understand. But doesn't it need a extra driver to set the
> > > > > switching
> > > > > frequency because of the new compatible string?
> >
> > > > I don't understand this bit at all, sorry.
> >
> > > Sebastian suggested a new sub-node in the devicetree:
> >
> > > powerpll {
> > > compatible = "fsl,imx28-power-pll"
> > > #clock-cells = <0>;
> > > clock-frequency = <12345>;
> > > }
> >
> > > and i think that the new compatible string needs a separate driver to take
> > > care
> > > of the switching frequency.
> >
> > I can see that this has been suggested, what I can't understand is why
> > we would wish to do this but I am missing some context here which may
> > make it all perfectly clear.
> 
> The relevant register DC-DC Miscellaneous Register (HW_POWER_MISC) is described
> in chapter 11.12.10 of the reference manual [1].
>
> [1] - http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf

Having a look at the datasheet my suggestion for the sub-node
doesn't make sense. I missunderstood how the hardware architecture
looks like, sorry for the noise :(

> changing the switching frequency is not a common feature for this IC. In most
> cases the clock for the DC-DC switching frequency runs at 24 MHz (ref_xtal).
> Unfortunatelly this could cause interferences for example in UHF band. So
> switching to ref_pll and changing clock divider is a option to avoid such
> interferences without changing hardware.
>
> I decided to implement it as a module parameter, because i see it's a
> configuration option.

I think its best to use a DT property then. That way it can be set
appropiatly for boards known to use a band, which could have
interference problems.

So just add a simple property (without a subnode). If I understand
Mark correctly, there's a high chance more drivers will need to
describe the switching frequency, so I suggest to use a vendor
independent property name:

switching-frequency = <12345>;

-- Sebastian

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 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 RFC 2/3] power: mxs_power: add driver for mxs power subsystem

[Sebastian Reichel]

Hi,

On Wed, Jan 28, 2015 at 11:22:05PM +0100, Stefan Wahren wrote:
> > Mark Brown <broonie@kernel.org> hat am 27. Januar 2015 um 20:43 geschrieben:
> > On Tue, Jan 27, 2015 at 07:35:09PM +0100, Stefan Wahren wrote:
> > > > Mark Brown <broonie@kernel.org> hat am 27. Januar 2015 um 01:16
> > > > geschrieben:
> >
> > > > > okay i understand. But doesn't it need a extra driver to set the
> > > > > switching
> > > > > frequency because of the new compatible string?
> >
> > > > I don't understand this bit at all, sorry.
> >
> > > Sebastian suggested a new sub-node in the devicetree:
> >
> > > powerpll {
> > > compatible = "fsl,imx28-power-pll"
> > > #clock-cells = <0>;
> > > clock-frequency = <12345>;
> > > }
> >
> > > and i think that the new compatible string needs a separate driver to take
> > > care
> > > of the switching frequency.
> >
> > I can see that this has been suggested, what I can't understand is why
> > we would wish to do this but I am missing some context here which may
> > make it all perfectly clear.
> 
> The relevant register DC-DC Miscellaneous Register (HW_POWER_MISC) is described
> in chapter 11.12.10 of the reference manual [1].
>
> [1] - http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf

Having a look at the datasheet my suggestion for the sub-node
doesn't make sense. I missunderstood how the hardware architecture
looks like, sorry for the noise :(

> changing the switching frequency is not a common feature for this IC. In most
> cases the clock for the DC-DC switching frequency runs at 24 MHz (ref_xtal).
> Unfortunatelly this could cause interferences for example in UHF band. So
> switching to ref_pll and changing clock divider is a option to avoid such
> interferences without changing hardware.
>
> I decided to implement it as a module parameter, because i see it's a
> configuration option.

I think its best to use a DT property then. That way it can be set
appropiatly for boards known to use a band, which could have
interference problems.

So just add a simple property (without a subnode). If I understand
Mark correctly, there's a high chance more drivers will need to
describe the switching frequency, so I suggest to use a vendor
independent property name:

switching-frequency = <12345>;

-- 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/20150128/2606ebdb/attachment-0001.sig>

Re: [PATCH RFC 2/3] power: mxs_power: add driver for mxs power subsystem

[Stefan Wahren]

Hi Sebastian,

> Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> hat am 28. Januar 2015 um 23:59
> geschrieben:
>
>
> Hi,
>
> On Wed, Jan 28, 2015 at 11:22:05PM +0100, Stefan Wahren wrote:
> > > Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> hat am 27. Januar 2015 um 20:43
> > > geschrieben:
> > > On Tue, Jan 27, 2015 at 07:35:09PM +0100, Stefan Wahren wrote:
> > >
> > changing the switching frequency is not a common feature for this IC. In
> > most
> > cases the clock for the DC-DC switching frequency runs at 24 MHz (ref_xtal).
> > Unfortunatelly this could cause interferences for example in UHF band. So
> > switching to ref_pll and changing clock divider is a option to avoid such
> > interferences without changing hardware.
> >
> > I decided to implement it as a module parameter, because i see it's a
> > configuration option.
>
> I think its best to use a DT property then. That way it can be set
> appropiatly for boards known to use a band, which could have
> interference problems.
>
> So just add a simple property (without a subnode). If I understand
> Mark correctly, there's a high chance more drivers will need to
> describe the switching frequency, so I suggest to use a vendor
> independent property name:
>
> switching-frequency = <12345>;

i'm not sure. So here is the complete current state (power and regulator binding
from imx28.dtsi) including my suggestion for the new property:

	power: power@80044000 {
		compatible = "fsl,imx28-power"; /* handled by mxs_power.c */
		#address-cells = <1>;
		#size-cells = <1>;
		reg = <0x80044000 0x2000>;
		interrupts = <6>;
		switching-frequency = <24000000>; /* new property */
		ranges;

		reg_vddd: regulator@80044040 {
			reg = <0x80044040 0x10>,
			      <0x80044010 0x10>,
			      <0x800440c0 0x10>;
			reg-names = "base-address",
				    "v5ctrl-address",
				    "status-address";
			compatible = "fsl,imx28-vddd"; /* handled by mxs-regulator.c */
			regulator-name = "vddd";
			regulator-min-microvolt = <1350000>;
			regulator-max-microvolt = <1550000>;
			vddd-supply = <&reg_vdda>;
		};

		reg_vdda: regulator@80044050 {
			reg = <0x80044050 0x10>,
			      <0x80044010 0x10>,
			      <0x800440c0 0x10>;
			reg-names = "base-address",
				    "v5ctrl-address",
				    "status-address";
			compatible = "fsl,imx28-vdda"; /* handled by mxs-regulator.c */
			regulator-name = "vdda";
			regulator-min-microvolt = <1725000>;
			regulator-max-microvolt = <1950000>;
			vdda-supply = <&reg_vddio>;
		};

		reg_vddio: regulator@80044060 {
			reg = <0x80044060 0x10>,
			      <0x80044010 0x10>,
			      <0x800440c0 0x10>;
			reg-names = "base-address",
				    "v5ctrl-address",
				    "status-address";
			compatible = "fsl,imx28-vddio"; /* handled by mxs-regulator.c */
			regulator-name = "vddio";
			regulator-min-microvolt = <3000000>;
			regulator-max-microvolt = <3575000>;
			regulator-microvolt-offset = <80000>;
		};
	};

Please correct me if i'm wrong.

Stefan

>
> -- Sebastian
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
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 RFC 2/3] power: mxs_power: add driver for mxs power subsystem

[Stefan Wahren]

Hi Sebastian,

> Sebastian Reichel <sre@kernel.org> hat am 28. Januar 2015 um 23:59
> geschrieben:
>
>
> Hi,
>
> On Wed, Jan 28, 2015 at 11:22:05PM +0100, Stefan Wahren wrote:
> > > Mark Brown <broonie@kernel.org> hat am 27. Januar 2015 um 20:43
> > > geschrieben:
> > > On Tue, Jan 27, 2015 at 07:35:09PM +0100, Stefan Wahren wrote:
> > >
> > changing the switching frequency is not a common feature for this IC. In
> > most
> > cases the clock for the DC-DC switching frequency runs at 24 MHz (ref_xtal).
> > Unfortunatelly this could cause interferences for example in UHF band. So
> > switching to ref_pll and changing clock divider is a option to avoid such
> > interferences without changing hardware.
> >
> > I decided to implement it as a module parameter, because i see it's a
> > configuration option.
>
> I think its best to use a DT property then. That way it can be set
> appropiatly for boards known to use a band, which could have
> interference problems.
>
> So just add a simple property (without a subnode). If I understand
> Mark correctly, there's a high chance more drivers will need to
> describe the switching frequency, so I suggest to use a vendor
> independent property name:
>
> switching-frequency = <12345>;

i'm not sure. So here is the complete current state (power and regulator binding
from imx28.dtsi) including my suggestion for the new property:

	power: power at 80044000 {
		compatible = "fsl,imx28-power"; /* handled by mxs_power.c */
		#address-cells = <1>;
		#size-cells = <1>;
		reg = <0x80044000 0x2000>;
		interrupts = <6>;
		switching-frequency = <24000000>; /* new property */
		ranges;

		reg_vddd: regulator at 80044040 {
			reg = <0x80044040 0x10>,
			      <0x80044010 0x10>,
			      <0x800440c0 0x10>;
			reg-names = "base-address",
				    "v5ctrl-address",
				    "status-address";
			compatible = "fsl,imx28-vddd"; /* handled by mxs-regulator.c */
			regulator-name = "vddd";
			regulator-min-microvolt = <1350000>;
			regulator-max-microvolt = <1550000>;
			vddd-supply = <&reg_vdda>;
		};

		reg_vdda: regulator at 80044050 {
			reg = <0x80044050 0x10>,
			      <0x80044010 0x10>,
			      <0x800440c0 0x10>;
			reg-names = "base-address",
				    "v5ctrl-address",
				    "status-address";
			compatible = "fsl,imx28-vdda"; /* handled by mxs-regulator.c */
			regulator-name = "vdda";
			regulator-min-microvolt = <1725000>;
			regulator-max-microvolt = <1950000>;
			vdda-supply = <&reg_vddio>;
		};

		reg_vddio: regulator at 80044060 {
			reg = <0x80044060 0x10>,
			      <0x80044010 0x10>,
			      <0x800440c0 0x10>;
			reg-names = "base-address",
				    "v5ctrl-address",
				    "status-address";
			compatible = "fsl,imx28-vddio"; /* handled by mxs-regulator.c */
			regulator-name = "vddio";
			regulator-min-microvolt = <3000000>;
			regulator-max-microvolt = <3575000>;
			regulator-microvolt-offset = <80000>;
		};
	};

Please correct me if i'm wrong.

Stefan

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

Re: [PATCH RFC 2/3] power: mxs_power: add driver for mxs power subsystem

[Stefan Wahren]

> Stefan Wahren <stefan.wahren@i2se.com> hat am 3. Februar 2015 um 21:41
> geschrieben:
>
>
> Hi Sebastian,
>
> > Sebastian Reichel <sre@kernel.org> hat am 28. Januar 2015 um 23:59
> > geschrieben:
> >
> >
> > Hi,
> >
> > On Wed, Jan 28, 2015 at 11:22:05PM +0100, Stefan Wahren wrote:
> > > > Mark Brown <broonie@kernel.org> hat am 27. Januar 2015 um 20:43
> > > > geschrieben:
> > > > On Tue, Jan 27, 2015 at 07:35:09PM +0100, Stefan Wahren wrote:
> > > >
> > > changing the switching frequency is not a common feature for this IC. In
> > > most
> > > cases the clock for the DC-DC switching frequency runs at 24 MHz
> > > (ref_xtal).
> > > Unfortunatelly this could cause interferences for example in UHF band. So
> > > switching to ref_pll and changing clock divider is a option to avoid such
> > > interferences without changing hardware.
> > >
> > > I decided to implement it as a module parameter, because i see it's a
> > > configuration option.
> >
> > I think its best to use a DT property then. That way it can be set
> > appropiatly for boards known to use a band, which could have
> > interference problems.
> >
> > So just add a simple property (without a subnode). If I understand
> > Mark correctly, there's a high chance more drivers will need to
> > describe the switching frequency, so I suggest to use a vendor
> > independent property name:
> >
> > switching-frequency = <12345>;
>
> i'm not sure. So here is the complete current state (power and regulator
> binding
> from imx28.dtsi) including my suggestion for the new property:
>
> power: power@80044000 {
> compatible = "fsl,imx28-power"; /* handled by mxs_power.c */
> #address-cells = <1>;
> #size-cells = <1>;
> reg = <0x80044000 0x2000>;
> interrupts = <6>;
> switching-frequency = <24000000>; /* new property */
> ranges;
>
> reg_vddd: regulator@80044040 {
> reg = <0x80044040 0x10>,
> <0x80044010 0x10>,
> <0x800440c0 0x10>;
> reg-names = "base-address",
> "v5ctrl-address",
> "status-address";
> compatible = "fsl,imx28-vddd"; /* handled by mxs-regulator.c */
> regulator-name = "vddd";
> regulator-min-microvolt = <1350000>;
> regulator-max-microvolt = <1550000>;
> vddd-supply = <&reg_vdda>;
> };
>
> reg_vdda: regulator@80044050 {
> reg = <0x80044050 0x10>,
> <0x80044010 0x10>,
> <0x800440c0 0x10>;
> reg-names = "base-address",
> "v5ctrl-address",
> "status-address";
> compatible = "fsl,imx28-vdda"; /* handled by mxs-regulator.c */
> regulator-name = "vdda";
> regulator-min-microvolt = <1725000>;
> regulator-max-microvolt = <1950000>;
> vdda-supply = <&reg_vddio>;
> };
>
> reg_vddio: regulator@80044060 {
> reg = <0x80044060 0x10>,
> <0x80044010 0x10>,
> <0x800440c0 0x10>;
> reg-names = "base-address",
> "v5ctrl-address",
> "status-address";
> compatible = "fsl,imx28-vddio"; /* handled by mxs-regulator.c */
> regulator-name = "vddio";
> regulator-min-microvolt = <3000000>;
> regulator-max-microvolt = <3575000>;
> regulator-microvolt-offset = <80000>;
> };
> };
>
> Please correct me if i'm wrong.
>
> Stefan

Gentle Ping.

Stefan

>
> >
> > -- Sebastian

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

[Stefan Wahren]

> Stefan Wahren <stefan.wahren@i2se.com> hat am 3. Februar 2015 um 21:41
> geschrieben:
>
>
> Hi Sebastian,
>
> > Sebastian Reichel <sre@kernel.org> hat am 28. Januar 2015 um 23:59
> > geschrieben:
> >
> >
> > Hi,
> >
> > On Wed, Jan 28, 2015 at 11:22:05PM +0100, Stefan Wahren wrote:
> > > > Mark Brown <broonie@kernel.org> hat am 27. Januar 2015 um 20:43
> > > > geschrieben:
> > > > On Tue, Jan 27, 2015 at 07:35:09PM +0100, Stefan Wahren wrote:
> > > >
> > > changing the switching frequency is not a common feature for this IC. In
> > > most
> > > cases the clock for the DC-DC switching frequency runs at 24 MHz
> > > (ref_xtal).
> > > Unfortunatelly this could cause interferences for example in UHF band. So
> > > switching to ref_pll and changing clock divider is a option to avoid such
> > > interferences without changing hardware.
> > >
> > > I decided to implement it as a module parameter, because i see it's a
> > > configuration option.
> >
> > I think its best to use a DT property then. That way it can be set
> > appropiatly for boards known to use a band, which could have
> > interference problems.
> >
> > So just add a simple property (without a subnode). If I understand
> > Mark correctly, there's a high chance more drivers will need to
> > describe the switching frequency, so I suggest to use a vendor
> > independent property name:
> >
> > switching-frequency = <12345>;
>
> i'm not sure. So here is the complete current state (power and regulator
> binding
> from imx28.dtsi) including my suggestion for the new property:
>
> power: power at 80044000 {
> compatible = "fsl,imx28-power"; /* handled by mxs_power.c */
> #address-cells = <1>;
> #size-cells = <1>;
> reg = <0x80044000 0x2000>;
> interrupts = <6>;
> switching-frequency = <24000000>; /* new property */
> ranges;
>
> reg_vddd: regulator at 80044040 {
> reg = <0x80044040 0x10>,
> <0x80044010 0x10>,
> <0x800440c0 0x10>;
> reg-names = "base-address",
> "v5ctrl-address",
> "status-address";
> compatible = "fsl,imx28-vddd"; /* handled by mxs-regulator.c */
> regulator-name = "vddd";
> regulator-min-microvolt = <1350000>;
> regulator-max-microvolt = <1550000>;
> vddd-supply = <&reg_vdda>;
> };
>
> reg_vdda: regulator at 80044050 {
> reg = <0x80044050 0x10>,
> <0x80044010 0x10>,
> <0x800440c0 0x10>;
> reg-names = "base-address",
> "v5ctrl-address",
> "status-address";
> compatible = "fsl,imx28-vdda"; /* handled by mxs-regulator.c */
> regulator-name = "vdda";
> regulator-min-microvolt = <1725000>;
> regulator-max-microvolt = <1950000>;
> vdda-supply = <&reg_vddio>;
> };
>
> reg_vddio: regulator at 80044060 {
> reg = <0x80044060 0x10>,
> <0x80044010 0x10>,
> <0x800440c0 0x10>;
> reg-names = "base-address",
> "v5ctrl-address",
> "status-address";
> compatible = "fsl,imx28-vddio"; /* handled by mxs-regulator.c */
> regulator-name = "vddio";
> regulator-min-microvolt = <3000000>;
> regulator-max-microvolt = <3575000>;
> regulator-microvolt-offset = <80000>;
> };
> };
>
> Please correct me if i'm wrong.
>
> Stefan

Gentle Ping.

Stefan

>
> >
> > -- Sebastian

Re: [PATCH RFC 2/3] power: mxs_power: add driver for mxs power subsystem

[Sebastian Reichel]

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

On Fri, Feb 20, 2015 at 11:57:41AM +0100, Stefan Wahren wrote:
> > power: power@80044000 {
> > compatible = "fsl,imx28-power"; /* handled by mxs_power.c */
> > #address-cells = <1>;
> > #size-cells = <1>;
> > reg = <0x80044000 0x2000>;
> > interrupts = <6>;
> > switching-frequency = <24000000>; /* new property */
> > ranges;
> >
> > reg_vddd: regulator@80044040 {
> > reg = <0x80044040 0x10>,
> > <0x80044010 0x10>,
> > <0x800440c0 0x10>;
> > reg-names = "base-address",
> > "v5ctrl-address",
> > "status-address";
> > compatible = "fsl,imx28-vddd"; /* handled by mxs-regulator.c */
> > regulator-name = "vddd";
> > regulator-min-microvolt = <1350000>;
> > regulator-max-microvolt = <1550000>;
> > vddd-supply = <&reg_vdda>;
> > };
> >
> > reg_vdda: regulator@80044050 {
> > reg = <0x80044050 0x10>,
> > <0x80044010 0x10>,
> > <0x800440c0 0x10>;
> > reg-names = "base-address",
> > "v5ctrl-address",
> > "status-address";
> > compatible = "fsl,imx28-vdda"; /* handled by mxs-regulator.c */
> > regulator-name = "vdda";
> > regulator-min-microvolt = <1725000>;
> > regulator-max-microvolt = <1950000>;
> > vdda-supply = <&reg_vddio>;
> > };
> >
> > reg_vddio: regulator@80044060 {
> > reg = <0x80044060 0x10>,
> > <0x80044010 0x10>,
> > <0x800440c0 0x10>;
> > reg-names = "base-address",
> > "v5ctrl-address",
> > "status-address";
> > compatible = "fsl,imx28-vddio"; /* handled by mxs-regulator.c */
> > regulator-name = "vddio";
> > regulator-min-microvolt = <3000000>;
> > regulator-max-microvolt = <3575000>;
> > regulator-microvolt-offset = <80000>;
> > };
> > };
> >
> > Please correct me if i'm wrong.
> >
> > Stefan
> 
> Gentle Ping.

The binding looks ok to me. It would be nice to get an ACK of a DT
binding maintainer, though.

-- Sebastian

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

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

[Sebastian Reichel]

On Fri, Feb 20, 2015 at 11:57:41AM +0100, Stefan Wahren wrote:
> > power: power at 80044000 {
> > compatible = "fsl,imx28-power"; /* handled by mxs_power.c */
> > #address-cells = <1>;
> > #size-cells = <1>;
> > reg = <0x80044000 0x2000>;
> > interrupts = <6>;
> > switching-frequency = <24000000>; /* new property */
> > ranges;
> >
> > reg_vddd: regulator at 80044040 {
> > reg = <0x80044040 0x10>,
> > <0x80044010 0x10>,
> > <0x800440c0 0x10>;
> > reg-names = "base-address",
> > "v5ctrl-address",
> > "status-address";
> > compatible = "fsl,imx28-vddd"; /* handled by mxs-regulator.c */
> > regulator-name = "vddd";
> > regulator-min-microvolt = <1350000>;
> > regulator-max-microvolt = <1550000>;
> > vddd-supply = <&reg_vdda>;
> > };
> >
> > reg_vdda: regulator at 80044050 {
> > reg = <0x80044050 0x10>,
> > <0x80044010 0x10>,
> > <0x800440c0 0x10>;
> > reg-names = "base-address",
> > "v5ctrl-address",
> > "status-address";
> > compatible = "fsl,imx28-vdda"; /* handled by mxs-regulator.c */
> > regulator-name = "vdda";
> > regulator-min-microvolt = <1725000>;
> > regulator-max-microvolt = <1950000>;
> > vdda-supply = <&reg_vddio>;
> > };
> >
> > reg_vddio: regulator at 80044060 {
> > reg = <0x80044060 0x10>,
> > <0x80044010 0x10>,
> > <0x800440c0 0x10>;
> > reg-names = "base-address",
> > "v5ctrl-address",
> > "status-address";
> > compatible = "fsl,imx28-vddio"; /* handled by mxs-regulator.c */
> > regulator-name = "vddio";
> > regulator-min-microvolt = <3000000>;
> > regulator-max-microvolt = <3575000>;
> > regulator-microvolt-offset = <80000>;
> > };
> > };
> >
> > Please correct me if i'm wrong.
> >
> > Stefan
> 
> Gentle Ping.

The binding looks ok to me. It would be nice to get an ACK of a DT
binding maintainer, though.

-- 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/20150223/6713c0a5/attachment-0001.sig>

Re: [PATCH RFC 2/3] power: mxs_power: add driver for mxs power subsystem

[Stefan Wahren]

Hi,

> Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> hat am 23. Februar 2015 um 16:38
> geschrieben:
>
>
> On Fri, Feb 20, 2015 at 11:57:41AM +0100, Stefan Wahren wrote:
> > > power: power@80044000 {
> > > compatible = "fsl,imx28-power"; /* handled by mxs_power.c */
> > > #address-cells = <1>;
> > > #size-cells = <1>;
> > > reg = <0x80044000 0x2000>;
> > > interrupts = <6>;
> > > switching-frequency = <24000000>; /* new property */
> > > ranges;
> > >
> > > reg_vddd: regulator@80044040 {
> > > reg = <0x80044040 0x10>,
> > > <0x80044010 0x10>,
> > > <0x800440c0 0x10>;
> > > reg-names = "base-address",
> > > "v5ctrl-address",
> > > "status-address";
> > > compatible = "fsl,imx28-vddd"; /* handled by mxs-regulator.c */
> > > regulator-name = "vddd";
> > > regulator-min-microvolt = <1350000>;
> > > regulator-max-microvolt = <1550000>;
> > > vddd-supply = <&reg_vdda>;
> > > };
> > >
> > > reg_vdda: regulator@80044050 {
> > > reg = <0x80044050 0x10>,
> > > <0x80044010 0x10>,
> > > <0x800440c0 0x10>;
> > > reg-names = "base-address",
> > > "v5ctrl-address",
> > > "status-address";
> > > compatible = "fsl,imx28-vdda"; /* handled by mxs-regulator.c */
> > > regulator-name = "vdda";
> > > regulator-min-microvolt = <1725000>;
> > > regulator-max-microvolt = <1950000>;
> > > vdda-supply = <&reg_vddio>;
> > > };
> > >
> > > reg_vddio: regulator@80044060 {
> > > reg = <0x80044060 0x10>,
> > > <0x80044010 0x10>,
> > > <0x800440c0 0x10>;
> > > reg-names = "base-address",
> > > "v5ctrl-address",
> > > "status-address";
> > > compatible = "fsl,imx28-vddio"; /* handled by mxs-regulator.c */
> > > regulator-name = "vddio";
> > > regulator-min-microvolt = <3000000>;
> > > regulator-max-microvolt = <3575000>;
> > > regulator-microvolt-offset = <80000>;
> > > };
> > > };
> > >
> > > Please correct me if i'm wrong.
> > >
> > > Stefan
> >
> > Gentle Ping.
>
> The binding looks ok to me. It would be nice to get an ACK of a DT
> binding maintainer, though.

any objections about implementing DC-DC switching frequency as a DT property
instead of module parameter?

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 RFC 2/3] power: mxs_power: add driver for mxs power subsystem

[Stefan Wahren]

Hi,

> Sebastian Reichel <sre@kernel.org> hat am 23. Februar 2015 um 16:38
> geschrieben:
>
>
> On Fri, Feb 20, 2015 at 11:57:41AM +0100, Stefan Wahren wrote:
> > > power: power at 80044000 {
> > > compatible = "fsl,imx28-power"; /* handled by mxs_power.c */
> > > #address-cells = <1>;
> > > #size-cells = <1>;
> > > reg = <0x80044000 0x2000>;
> > > interrupts = <6>;
> > > switching-frequency = <24000000>; /* new property */
> > > ranges;
> > >
> > > reg_vddd: regulator at 80044040 {
> > > reg = <0x80044040 0x10>,
> > > <0x80044010 0x10>,
> > > <0x800440c0 0x10>;
> > > reg-names = "base-address",
> > > "v5ctrl-address",
> > > "status-address";
> > > compatible = "fsl,imx28-vddd"; /* handled by mxs-regulator.c */
> > > regulator-name = "vddd";
> > > regulator-min-microvolt = <1350000>;
> > > regulator-max-microvolt = <1550000>;
> > > vddd-supply = <&reg_vdda>;
> > > };
> > >
> > > reg_vdda: regulator at 80044050 {
> > > reg = <0x80044050 0x10>,
> > > <0x80044010 0x10>,
> > > <0x800440c0 0x10>;
> > > reg-names = "base-address",
> > > "v5ctrl-address",
> > > "status-address";
> > > compatible = "fsl,imx28-vdda"; /* handled by mxs-regulator.c */
> > > regulator-name = "vdda";
> > > regulator-min-microvolt = <1725000>;
> > > regulator-max-microvolt = <1950000>;
> > > vdda-supply = <&reg_vddio>;
> > > };
> > >
> > > reg_vddio: regulator at 80044060 {
> > > reg = <0x80044060 0x10>,
> > > <0x80044010 0x10>,
> > > <0x800440c0 0x10>;
> > > reg-names = "base-address",
> > > "v5ctrl-address",
> > > "status-address";
> > > compatible = "fsl,imx28-vddio"; /* handled by mxs-regulator.c */
> > > regulator-name = "vddio";
> > > regulator-min-microvolt = <3000000>;
> > > regulator-max-microvolt = <3575000>;
> > > regulator-microvolt-offset = <80000>;
> > > };
> > > };
> > >
> > > Please correct me if i'm wrong.
> > >
> > > Stefan
> >
> > Gentle Ping.
>
> The binding looks ok to me. It would be nice to get an ACK of a DT
> binding maintainer, though.

any objections about implementing DC-DC switching frequency as a DT property
instead of module parameter?

Stefan

[PATCH RFC 3/3] ARM: dts: enable power subsystem for i.MX28

[Stefan Wahren]

This patch enables the power subsystem for all i.MX28 boards.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 arch/arm/boot/dts/imx28.dtsi |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index 47f68ac..5c163a5 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -1022,8 +1022,12 @@
 			};

 			power: power@80044000 {
+				compatible = "fsl,imx28-power";
+				#address-cells = <1>;
+				#size-cells = <1>;
 				reg = <0x80044000 0x2000>;
-				status = "disabled";
+				interrupts = <6>;
+				ranges;
 			};

 			saif1: saif@80046000 {
--
1.7.9.5

[PATCH RFC 3/3] ARM: dts: enable power subsystem for i.MX28

[Stefan Wahren]

This patch enables the power subsystem for all i.MX28 boards.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
---
 arch/arm/boot/dts/imx28.dtsi |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index 47f68ac..5c163a5 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -1022,8 +1022,12 @@
 			};

 			power: power at 80044000 {
+				compatible = "fsl,imx28-power";
+				#address-cells = <1>;
+				#size-cells = <1>;
 				reg = <0x80044000 0x2000>;
-				status = "disabled";
+				interrupts = <6>;
+				ranges;
 			};

 			saif1: saif at 80046000 {
--
1.7.9.5

Re: [PATCH RFC 3/3] ARM: dts: enable power subsystem for i.MX28

[Fabio Estevam]

Hi Stefan,

On Thu, Nov 20, 2014 at 6:14 PM, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> This patch enables the power subsystem for all i.MX28 boards.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
>  arch/arm/boot/dts/imx28.dtsi |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> index 47f68ac..5c163a5 100644
> --- a/arch/arm/boot/dts/imx28.dtsi
> +++ b/arch/arm/boot/dts/imx28.dtsi
> @@ -1022,8 +1022,12 @@
>                         };
>
>                         power: power@80044000 {
> +                               compatible = "fsl,imx28-power";

Should we have 'fsl,imx23-power' and 'fsl,imx28-power' as compatible strings?

Or  ''fsl,mxs-power' could be used for both?

[PATCH RFC 3/3] ARM: dts: enable power subsystem for i.MX28

[Fabio Estevam]

Hi Stefan,

On Thu, Nov 20, 2014 at 6:14 PM, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> This patch enables the power subsystem for all i.MX28 boards.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
>  arch/arm/boot/dts/imx28.dtsi |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> index 47f68ac..5c163a5 100644
> --- a/arch/arm/boot/dts/imx28.dtsi
> +++ b/arch/arm/boot/dts/imx28.dtsi
> @@ -1022,8 +1022,12 @@
>                         };
>
>                         power: power at 80044000 {
> +                               compatible = "fsl,imx28-power";

Should we have 'fsl,imx23-power' and 'fsl,imx28-power' as compatible strings?

Or  ''fsl,mxs-power' could be used for both?

Re: [PATCH RFC 3/3] ARM: dts: enable power subsystem for i.MX28

[Stefan Wahren]

Hi Fabio,

> Fabio Estevam <festevam@gmail.com> hat am 20. November 2014 um 22:32
> geschrieben:
>
>
> Hi Stefan,
>
> On Thu, Nov 20, 2014 at 6:14 PM, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> > This patch enables the power subsystem for all i.MX28 boards.
> >
> > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> > ---
> > arch/arm/boot/dts/imx28.dtsi | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> > index 47f68ac..5c163a5 100644
> > --- a/arch/arm/boot/dts/imx28.dtsi
> > +++ b/arch/arm/boot/dts/imx28.dtsi
> > @@ -1022,8 +1022,12 @@
> > };
> >
> > power: power@80044000 {
> > + compatible = "fsl,imx28-power";
>
> Should we have 'fsl,imx23-power' and 'fsl,imx28-power' as compatible strings?

yes, of course.

>
> Or ''fsl,mxs-power' could be used for both?

I think that would be a bad idea. I can't believe that all PMU register of
i.MX23 and i.MX28 are identical. So in the future we will have to distinguish
between them. It's better to do it now like most of the mxs driver does.

Stefan

[PATCH RFC 3/3] ARM: dts: enable power subsystem for i.MX28

[Stefan Wahren]

Hi Fabio,

> Fabio Estevam <festevam@gmail.com> hat am 20. November 2014 um 22:32
> geschrieben:
>
>
> Hi Stefan,
>
> On Thu, Nov 20, 2014 at 6:14 PM, Stefan Wahren <stefan.wahren@i2se.com> wrote:
> > This patch enables the power subsystem for all i.MX28 boards.
> >
> > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> > ---
> > arch/arm/boot/dts/imx28.dtsi | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> > index 47f68ac..5c163a5 100644
> > --- a/arch/arm/boot/dts/imx28.dtsi
> > +++ b/arch/arm/boot/dts/imx28.dtsi
> > @@ -1022,8 +1022,12 @@
> > };
> >
> > power: power at 80044000 {
> > + compatible = "fsl,imx28-power";
>
> Should we have 'fsl,imx23-power' and 'fsl,imx28-power' as compatible strings?

yes, of course.

>
> Or ''fsl,mxs-power' could be used for both?

I think that would be a bad idea. I can't believe that all PMU register of
i.MX23 and i.MX28 are identical. So in the future we will have to distinguish
between them. It's better to do it now like most of the mxs driver does.

Stefan

Re: [PATCH RFC 3/3] ARM: dts: enable power subsystem for i.MX28

[Fabio Estevam]

On Thu, Nov 20, 2014 at 8:57 PM, Stefan Wahren <stefan.wahren@i2se.com> wrote:

>> Or ''fsl,mxs-power' could be used for both?
>
> I think that would be a bad idea. I can't believe that all PMU register of
> i.MX23 and i.MX28 are identical. So in the future we will have to distinguish
> between them. It's better to do it now like most of the mxs driver does.


Ok, sounds good then.

[PATCH RFC 3/3] ARM: dts: enable power subsystem for i.MX28

[Fabio Estevam]

On Thu, Nov 20, 2014 at 8:57 PM, Stefan Wahren <stefan.wahren@i2se.com> wrote:

>> Or ''fsl,mxs-power' could be used for both?
>
> I think that would be a bad idea. I can't believe that all PMU register of
> i.MX23 and i.MX28 are identical. So in the future we will have to distinguish
> between them. It's better to do it now like most of the mxs driver does.


Ok, sounds good then.

Re: [PATCH RFC 0/3] power: mxs_power: add driver for mxs power subsystem

[Stefan Wahren]

> This patch series adds support for Freescale i.MX23, i.xM28 power subsystem.
>
> The idea for this driver was born in the discussion with Mark Rutland
> about the mxs-regulator driver [1].
>
> The main use is to trigger driver probing of the underlying DT child nodes
> like on-chip regulators. But it's also the first step for cpufreq-dt support.
>
> [1] - https://lkml.org/lkml/2014/9/29/283
>
> Stefan Wahren (3):
>   DT: add binding for mxs power subsystem
>   power: mxs_power: add driver for mxs power subsystem
>   ARM: dts: enable power subsystem for i.MX28
>
>  .../devicetree/bindings/power/mxs_power.txt        |   23 ++
>  arch/arm/boot/dts/imx28.dtsi                       |    6 +-
>  drivers/power/Kconfig                              |    8 +
>  drivers/power/Makefile                             |    1 +
>  drivers/power/mxs_power.c                          |  226 ++++++++++++++++++++
>  5 files changed, 263 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/power/mxs_power.txt
>  create mode 100644 drivers/power/mxs_power.c
>
> --
> 1.7.9.5
>

Are there any other questions or suggestions about these patch series?

Stefan

[PATCH RFC 0/3] power: mxs_power: add driver for mxs power subsystem

[Stefan Wahren]

> This patch series adds support for Freescale i.MX23, i.xM28 power subsystem.
>
> The idea for this driver was born in the discussion with Mark Rutland
> about the mxs-regulator driver [1].
>
> The main use is to trigger driver probing of the underlying DT child nodes
> like on-chip regulators. But it's also the first step for cpufreq-dt support.
>
> [1] - https://lkml.org/lkml/2014/9/29/283
>
> Stefan Wahren (3):
>   DT: add binding for mxs power subsystem
>   power: mxs_power: add driver for mxs power subsystem
>   ARM: dts: enable power subsystem for i.MX28
>
>  .../devicetree/bindings/power/mxs_power.txt        |   23 ++
>  arch/arm/boot/dts/imx28.dtsi                       |    6 +-
>  drivers/power/Kconfig                              |    8 +
>  drivers/power/Makefile                             |    1 +
>  drivers/power/mxs_power.c                          |  226 ++++++++++++++++++++
>  5 files changed, 263 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/power/mxs_power.txt
>  create mode 100644 drivers/power/mxs_power.c
>
> --
> 1.7.9.5
>

Are there any other questions or suggestions about these patch series?

Stefan