devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 1/2] regulator: max14577: Add regulator driver for Maxim 14577
@ 2013-11-27 13:46 Krzysztof Kozlowski
       [not found] ` <1385559982-32039-1-git-send-email-k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
  2013-11-27 15:20 ` Mark Brown
  0 siblings, 2 replies; 10+ messages in thread
From: Krzysztof Kozlowski @ 2013-11-27 13:46 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, Rob Landley, Samuel Ortiz, Lee Jones, Liam Girdwood,
	Mark Brown, devicetree, linux-doc, linux-kernel
  Cc: Kyungmin Park, Bartlomiej Zolnierkiewicz, Marek Szyprowski,
	Krzysztof Kozlowski

MAX14577 chip is a multi-function device which includes MUIC,
charger and voltage regulator. The driver is located in drivers/mfd.

This patch adds regulator driver for MAX14577 chip. There are two
regulators in this chip:
1. Safeout LDO with constant voltage output of 4.9V. It can be only
   enabled or disabled.
2. Current regulator for the charger. It provides current from 90mA up
   to 950mA.
Driver supports Device Tree.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/mfd/max14577.c       |   13 +-
 drivers/regulator/Kconfig    |    7 ++
 drivers/regulator/Makefile   |    1 +
 drivers/regulator/max14577.c |  269 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 287 insertions(+), 3 deletions(-)
 create mode 100644 drivers/regulator/max14577.c

diff --git a/drivers/mfd/max14577.c b/drivers/mfd/max14577.c
index 94b766d..9af46c2 100644
--- a/drivers/mfd/max14577.c
+++ b/drivers/mfd/max14577.c
@@ -25,9 +25,16 @@
 #include <linux/mfd/max14577-private.h>
 
 static struct mfd_cell max14577_devs[] = {
-	{ .name = "max14577-muic", },
-	{ .name = "max14577-regulator", },
-	{ .name = "max14577-charger", },
+	{
+		.name = "max14577-muic",
+	},
+	{
+		.name = "max14577-regulator",
+		.of_compatible = "maxim,max14577-regulator",
+	},
+	{
+		.name = "max14577-charger",
+	},
 };
 
 static bool max14577_volatile_reg(struct device *dev, unsigned int reg)
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index ce785f4..11ee053 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -249,6 +249,13 @@ config REGULATOR_LP8788
 	help
 	  This driver supports LP8788 voltage regulator chip.
 
+config REGULATOR_MAX14577
+	tristate "Maxim 14577 regulator"
+	depends on MFD_MAX14577
+	help
+	  This driver controls a Maxim 14577 regulator via I2C bus.
+	  The regulators include safeout LDO and current regulator 'CHARGER'.
+
 config REGULATOR_MAX1586
 	tristate "Maxim 1586/1587 voltage regulator"
 	depends on I2C
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 01c597e..654bd43 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -35,6 +35,7 @@ obj-$(CONFIG_REGULATOR_LP872X) += lp872x.o
 obj-$(CONFIG_REGULATOR_LP8788) += lp8788-buck.o
 obj-$(CONFIG_REGULATOR_LP8788) += lp8788-ldo.o
 obj-$(CONFIG_REGULATOR_LP8755) += lp8755.o
+obj-$(CONFIG_REGULATOR_MAX14577) += max14577.o
 obj-$(CONFIG_REGULATOR_MAX1586) += max1586.o
 obj-$(CONFIG_REGULATOR_MAX8649)	+= max8649.o
 obj-$(CONFIG_REGULATOR_MAX8660) += max8660.o
diff --git a/drivers/regulator/max14577.c b/drivers/regulator/max14577.c
new file mode 100644
index 0000000..400bdfc
--- /dev/null
+++ b/drivers/regulator/max14577.c
@@ -0,0 +1,269 @@
+/*
+ * max14577.c - Regulator driver for the Maxim 14577
+ *
+ * Copyright (C) 2013 Samsung Electronics
+ * Krzysztof Kozlowski <k.kozlowski@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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/module.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/mfd/max14577.h>
+#include <linux/mfd/max14577-private.h>
+#include <linux/regulator/of_regulator.h>
+
+struct max14577_regulator {
+	struct device *dev;
+	struct max14577 *max14577;
+	struct regulator_dev **regulators;
+};
+
+static int max14577_reg_is_enabled(struct regulator_dev *rdev)
+{
+	int rid = rdev_get_id(rdev);
+	struct regmap *rmap = rdev->regmap;
+	u8 reg_data;
+
+	switch (rid) {
+	case MAX14577_CHARGER:
+		max14577_read_reg(rmap, MAX14577_CHG_REG_CHG_CTRL2, &reg_data);
+		if ((reg_data & CHGCTRL2_MBCHOSTEN_MASK) == 0)
+			return 0;
+		max14577_read_reg(rmap, MAX14577_CHG_REG_STATUS3, &reg_data);
+		if ((reg_data & STATUS3_CGMBC_MASK) == 0)
+			return 0;
+		/* MBCHOSTEN and CGMBC are on */
+		return 1;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int max14577_reg_get_current_limit(struct regulator_dev *rdev)
+{
+	u8 reg_data;
+	struct regmap *rmap = rdev->regmap;
+
+	if (rdev_get_id(rdev) != MAX14577_CHARGER)
+		return -EINVAL;
+
+	max14577_read_reg(rmap, MAX14577_CHG_REG_CHG_CTRL4, &reg_data);
+
+	if ((reg_data & CHGCTRL4_MBCICHWRCL_MASK) == 0)
+		return MAX14577_REGULATOR_CURRENT_LIMIT_MIN;
+
+	reg_data = ((reg_data & CHGCTRL4_MBCICHWRCH_MASK) >>
+			CHGCTRL4_MBCICHWRCH_SHIFT);
+	return MAX14577_REGULATOR_CURRENT_LIMIT_HIGH_START +
+		reg_data * MAX14577_REGULATOR_CURRENT_LIMIT_HIGH_STEP;
+}
+
+static int max14577_reg_set_current_limit(struct regulator_dev *rdev,
+		int min_uA, int max_uA)
+{
+	int i, current_bits = 0xf;
+	u8 reg_data;
+
+	if (rdev_get_id(rdev) != MAX14577_CHARGER)
+		return -EINVAL;
+
+	if (min_uA > MAX14577_REGULATOR_CURRENT_LIMIT_MAX ||
+			max_uA < MAX14577_REGULATOR_CURRENT_LIMIT_MIN)
+		return -EINVAL;
+
+	if (max_uA < MAX14577_REGULATOR_CURRENT_LIMIT_HIGH_START) {
+		/* Less than 200 mA, so set 90mA (turn only Low Bit off) */
+		u8 reg_data = 0x0 << CHGCTRL4_MBCICHWRCL_SHIFT;
+		return max14577_update_reg(rdev->regmap,
+				MAX14577_CHG_REG_CHG_CTRL4,
+				CHGCTRL4_MBCICHWRCL_MASK, reg_data);
+	}
+
+	/* max_uA is in range: <LIMIT_HIGH_START, inifinite>, so search for
+	 * valid current starting from LIMIT_MAX. */
+	for (i = MAX14577_REGULATOR_CURRENT_LIMIT_MAX;
+			i >= MAX14577_REGULATOR_CURRENT_LIMIT_HIGH_START;
+			i -= MAX14577_REGULATOR_CURRENT_LIMIT_HIGH_STEP) {
+		if (i <= max_uA)
+			break;
+		current_bits--;
+	}
+	BUG_ON(current_bits < 0); /* Cannot happen */
+	/* Turn Low Bit on (use range 200mA-950 mA) */
+	reg_data = 0x1 << CHGCTRL4_MBCICHWRCL_SHIFT;
+	/* and set proper High Bits */
+	reg_data |= current_bits << CHGCTRL4_MBCICHWRCH_SHIFT;
+
+	return max14577_update_reg(rdev->regmap, MAX14577_CHG_REG_CHG_CTRL4,
+			CHGCTRL4_MBCICHWRCL_MASK | CHGCTRL4_MBCICHWRCH_MASK,
+			reg_data);
+}
+
+static struct regulator_ops max14577_safeout_ops = {
+	.is_enabled		= regulator_is_enabled_regmap,
+	.enable			= regulator_enable_regmap,
+	.disable		= regulator_disable_regmap,
+	.list_voltage		= regulator_list_voltage_linear,
+};
+
+static struct regulator_ops max14577_charger_ops = {
+	.is_enabled		= max14577_reg_is_enabled,
+	.enable			= regulator_enable_regmap,
+	.disable		= regulator_disable_regmap,
+	.get_current_limit	= max14577_reg_get_current_limit,
+	.set_current_limit	= max14577_reg_set_current_limit,
+};
+
+static const struct regulator_desc supported_regulators[] = {
+	[MAX14577_SAFEOUT] = {
+		.name		= "SAFEOUT",
+		.id		= MAX14577_SAFEOUT,
+		.ops		= &max14577_safeout_ops,
+		.type		= REGULATOR_VOLTAGE,
+		.owner		= THIS_MODULE,
+		.n_voltages	= 1,
+		.min_uV		= MAX14577_REGULATOR_SAFEOUT_VOLTAGE,
+		.enable_reg	= MAX14577_REG_CONTROL2,
+		.enable_mask	= CTRL2_SFOUTORD_MASK,
+	},
+	[MAX14577_CHARGER] = {
+		.name		= "CHARGER",
+		.id		= MAX14577_CHARGER,
+		.ops		= &max14577_charger_ops,
+		.type		= REGULATOR_CURRENT,
+		.owner		= THIS_MODULE,
+		.enable_reg	= MAX14577_CHG_REG_CHG_CTRL2,
+		.enable_mask	= CHGCTRL2_MBCHOSTEN_MASK,
+	},
+};
+
+#ifdef CONFIG_OF
+static struct of_regulator_match max14577_regulator_matches[] = {
+	{ .name	= "SAFEOUT", },
+	{ .name = "CHARGER", },
+};
+
+static int max14577_regulator_dt_parse_pdata(struct platform_device *pdev)
+{
+	int ret;
+	struct device_node *np = pdev->dev.of_node;
+
+	ret = of_regulator_match(&pdev->dev, np, max14577_regulator_matches,
+			MAX14577_REG_MAX);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Error parsing regulator init data: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static inline struct regulator_init_data *match_init_data(int index)
+{
+	return max14577_regulator_matches[index].init_data;
+}
+
+static inline struct device_node *match_of_node(int index)
+{
+	return max14577_regulator_matches[index].of_node;
+}
+#else /* CONFIG_OF */
+static int max14577_regulator_dt_parse_pdata(struct platform_device *pdev)
+{
+	return 0;
+}
+static inline struct regulator_init_data *match_init_data(int index)
+{
+	return NULL;
+}
+
+static inline struct device_node *match_of_node(int index)
+{
+	return NULL;
+}
+#endif /* CONFIG_OF */
+
+
+static int max14577_regulator_probe(struct platform_device *pdev)
+{
+	struct max14577 *max14577 = dev_get_drvdata(pdev->dev.parent);
+	struct device_node *np = pdev->dev.of_node;
+	struct max14577_platform_data *pdata = dev_get_platdata(max14577->dev);
+	int i;
+	struct regulator_config config = {};
+
+	if (np) {
+		int ret = max14577_regulator_dt_parse_pdata(pdev);
+		if (ret)
+			return ret;
+	}
+
+	config.dev = &pdev->dev;
+	config.regmap = max14577->regmap;
+
+	for (i = 0; i < ARRAY_SIZE(supported_regulators); i++) {
+		struct regulator_dev *regulator;
+		/*
+		 * Index of supported_regulators[] is also the id and must
+		 * match index of pdata->regulators[].
+		 */
+		if (pdata && pdata->regulators) {
+			config.init_data = pdata->regulators[i].initdata;
+			config.of_node = pdata->regulators[i].of_node;
+		} else {
+			config.init_data = match_init_data(i);
+			config.of_node = match_of_node(i);
+		}
+
+		regulator = devm_regulator_register(&pdev->dev,
+				&supported_regulators[i], &config);
+		if (IS_ERR(regulator)) {
+			int ret = PTR_ERR(regulator);
+			dev_err(&pdev->dev,
+					"Regulator init failed for ID %d with error: %d\n",
+					i, ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static struct platform_driver max14577_regulator_driver = {
+	.driver = {
+		   .owner = THIS_MODULE,
+		   .name = "max14577-regulator",
+		   },
+	.probe	= max14577_regulator_probe,
+};
+
+static int __init max14577_regulator_init(void)
+{
+	BUILD_BUG_ON(MAX14577_REGULATOR_CURRENT_LIMIT_HIGH_START +
+			MAX14577_REGULATOR_CURRENT_LIMIT_HIGH_STEP * 0xf !=
+			MAX14577_REGULATOR_CURRENT_LIMIT_MAX);
+	BUILD_BUG_ON(ARRAY_SIZE(supported_regulators) != MAX14577_REG_MAX);
+
+	return platform_driver_register(&max14577_regulator_driver);
+}
+subsys_initcall(max14577_regulator_init);
+
+static void __exit max14577_regulator_exit(void)
+{
+	platform_driver_unregister(&max14577_regulator_driver);
+}
+module_exit(max14577_regulator_exit);
+
+MODULE_AUTHOR("Krzysztof Kozlowski <k.kozlowski@samsung.com>");
+MODULE_DESCRIPTION("MAXIM 14577 regulator driver");
+MODULE_LICENSE("GPL");
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v5 2/2] mfd: max14577: Add device tree bindings document
       [not found] ` <1385559982-32039-1-git-send-email-k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
@ 2013-11-27 13:46   ` Krzysztof Kozlowski
  2013-11-27 13:59   ` [PATCH v5 1/2] regulator: max14577: Add regulator driver for Maxim 14577 Lee Jones
  1 sibling, 0 replies; 10+ messages in thread
From: Krzysztof Kozlowski @ 2013-11-27 13:46 UTC (permalink / raw)
  To: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, Rob Landley, Samuel Ortiz, Lee Jones, Liam Girdwood,
	Mark Brown, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Kyungmin Park, Bartlomiej Zolnierkiewicz, Marek Szyprowski,
	Krzysztof Kozlowski

Add document describing device tree bindings for MAX14577 MFD driver.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 Documentation/devicetree/bindings/mfd/max14577.txt |   48 ++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/max14577.txt

diff --git a/Documentation/devicetree/bindings/mfd/max14577.txt b/Documentation/devicetree/bindings/mfd/max14577.txt
new file mode 100644
index 0000000..f4fd163
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/max14577.txt
@@ -0,0 +1,48 @@
+MAXIM MAX14577 multi-function device
+
+MAX14577 is a Multi-function device with Micro-USB Interface Circuit, Li+
+Battery Charger and SFOUT LDO output for powering USB devices. It is
+interfaced to host controller using I2C.
+
+Required properties:
+- compatible : Must be "maxim,max14577".
+- reg : I2C slave address for the max14577 chip.
+- interrupts : IRQ line for the max14577 chip.
+- interrupt-parent :  The parent interrupt controller.
+
+Optional nodes:
+- regulators :
+  Required child node properties:
+  - compatible : "maxim,max14577-regulator"
+
+  Optional child nodes:
+    Each child node representing a regulator, following standard regulator
+    bindings. Valid names for a regulator are: "CHARGER" and "SAFEOUT".
+    The SAFEOUT is a constant voltage regulator so there is no need to specify
+    voltages for it.
+
+	[*] refer Documentation/devicetree/bindings/regulator/regulator.txt
+
+Example:
+	max14577@25 {
+		compatible = "maxim,max14577";
+		reg = <0x25>;
+		interrupt-parent = <&gpx1>;
+		interrupts = <5 0>;
+
+		regulators {
+			compatible = "maxim,max14577-regulator";
+
+			safeout_reg: safeout@1 {
+				regulator-compatible = "SAFEOUT";
+				regulator-name = "SAFEOUT";
+			};
+			charger_reg: charger@0 {
+				regulator-compatible = "CHARGER";
+				regulator-name = "CHARGER";
+				regulator-min-microamp = <90000>;
+				regulator-max-microamp = <950000>;
+				regulator-boot-on;
+			};
+		};
+	};
-- 
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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 1/2] regulator: max14577: Add regulator driver for Maxim 14577
       [not found] ` <1385559982-32039-1-git-send-email-k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
  2013-11-27 13:46   ` [PATCH v5 2/2] mfd: max14577: Add device tree bindings document Krzysztof Kozlowski
@ 2013-11-27 13:59   ` Lee Jones
  1 sibling, 0 replies; 10+ messages in thread
From: Lee Jones @ 2013-11-27 13:59 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, Rob Landley, Samuel Ortiz, Liam Girdwood,
	Mark Brown, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Kyungmin Park,
	Bartlomiej Zolnierkiewicz, Marek Szyprowski

On Wed, 27 Nov 2013, Krzysztof Kozlowski wrote:

> MAX14577 chip is a multi-function device which includes MUIC,
> charger and voltage regulator. The driver is located in drivers/mfd.
> 
> This patch adds regulator driver for MAX14577 chip. There are two
> regulators in this chip:
> 1. Safeout LDO with constant voltage output of 4.9V. It can be only
>    enabled or disabled.
> 2. Current regulator for the charger. It provides current from 90mA up
>    to 950mA.
> Driver supports Device Tree.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/mfd/max14577.c       |   13 +-
>  drivers/regulator/Kconfig    |    7 ++
>  drivers/regulator/Makefile   |    1 +
>  drivers/regulator/max14577.c |  269 ++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 287 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/regulator/max14577.c
> 
> diff --git a/drivers/mfd/max14577.c b/drivers/mfd/max14577.c
> index 94b766d..9af46c2 100644
> --- a/drivers/mfd/max14577.c
> +++ b/drivers/mfd/max14577.c
> @@ -25,9 +25,16 @@
>  #include <linux/mfd/max14577-private.h>
>  
>  static struct mfd_cell max14577_devs[] = {
> -	{ .name = "max14577-muic", },
> -	{ .name = "max14577-regulator", },
> -	{ .name = "max14577-charger", },
> +	{
> +		.name = "max14577-muic",
> +	},
> +	{
> +		.name = "max14577-regulator",
> +		.of_compatible = "maxim,max14577-regulator",
> +	},
> +	{
> +		.name = "max14577-charger",
> +	},
>  };

Still it still part of the regulator patch.

Can you pull the code above into a separate patch please?

Also, there's no requirement to open out all of the one liners, just
the entry where you're using two properties.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 1/2] regulator: max14577: Add regulator driver for Maxim 14577
  2013-11-27 13:46 [PATCH v5 1/2] regulator: max14577: Add regulator driver for Maxim 14577 Krzysztof Kozlowski
       [not found] ` <1385559982-32039-1-git-send-email-k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
@ 2013-11-27 15:20 ` Mark Brown
  2013-11-27 15:23   ` Lee Jones
  1 sibling, 1 reply; 10+ messages in thread
From: Mark Brown @ 2013-11-27 15:20 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, Rob Landley, Samuel Ortiz, Lee Jones, Liam Girdwood,
	devicetree, linux-doc, linux-kernel, Kyungmin Park,
	Bartlomiej Zolnierkiewicz, Marek Szyprowski

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

On Wed, Nov 27, 2013 at 02:46:21PM +0100, Krzysztof Kozlowski wrote:

> +	{
> +		.name = "max14577-regulator",
> +		.of_compatible = "maxim,max14577-regulator",
> +	},

Why is there a compatible for this at all, would this ever appear as
part of a different device?

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 1/2] regulator: max14577: Add regulator driver for Maxim 14577
  2013-11-27 15:20 ` Mark Brown
@ 2013-11-27 15:23   ` Lee Jones
  2013-11-27 15:42     ` Mark Brown
  0 siblings, 1 reply; 10+ messages in thread
From: Lee Jones @ 2013-11-27 15:23 UTC (permalink / raw)
  To: Mark Brown
  Cc: Krzysztof Kozlowski, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, Rob Landley, Samuel Ortiz,
	Liam Girdwood, devicetree, linux-doc, linux-kernel, Kyungmin Park,
	Bartlomiej Zolnierkiewicz, Marek Szyprowski

On Wed, 27 Nov 2013, Mark Brown wrote:

> On Wed, Nov 27, 2013 at 02:46:21PM +0100, Krzysztof Kozlowski wrote:
> 
> > +	{
> > +		.name = "max14577-regulator",
> > +		.of_compatible = "maxim,max14577-regulator",
> > +	},
> 
> Why is there a compatible for this at all, would this ever appear as
> part of a different device?

For automatic assigning of it's of_node.

In previous incarnations of this set it was done manually for some reason.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 1/2] regulator: max14577: Add regulator driver for Maxim 14577
  2013-11-27 15:23   ` Lee Jones
@ 2013-11-27 15:42     ` Mark Brown
  2013-11-27 16:13       ` Lee Jones
  0 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2013-11-27 15:42 UTC (permalink / raw)
  To: Lee Jones
  Cc: Krzysztof Kozlowski, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, Rob Landley, Samuel Ortiz,
	Liam Girdwood, devicetree, linux-doc, linux-kernel, Kyungmin Park,
	Bartlomiej Zolnierkiewicz, Marek Szyprowski

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

On Wed, Nov 27, 2013 at 03:23:58PM +0000, Lee Jones wrote:
> On Wed, 27 Nov 2013, Mark Brown wrote:
> > > +	{
> > > +		.name = "max14577-regulator",
> > > +		.of_compatible = "maxim,max14577-regulator",
> > > +	},

> > Why is there a compatible for this at all, would this ever appear as
> > part of a different device?

> For automatic assigning of it's of_node.

> In previous incarnations of this set it was done manually for some reason.

The usual thing to do is of_get_child_by_name() on the parent to get the
container to search in.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 1/2] regulator: max14577: Add regulator driver for Maxim 14577
  2013-11-27 15:42     ` Mark Brown
@ 2013-11-27 16:13       ` Lee Jones
  2013-11-27 16:28         ` Mark Brown
  0 siblings, 1 reply; 10+ messages in thread
From: Lee Jones @ 2013-11-27 16:13 UTC (permalink / raw)
  To: Mark Brown
  Cc: Krzysztof Kozlowski, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, Rob Landley, Samuel Ortiz,
	Liam Girdwood, devicetree, linux-doc, linux-kernel, Kyungmin Park,
	Bartlomiej Zolnierkiewicz, Marek Szyprowski

On Wed, 27 Nov 2013, Mark Brown wrote:

> On Wed, Nov 27, 2013 at 03:23:58PM +0000, Lee Jones wrote:
> > On Wed, 27 Nov 2013, Mark Brown wrote:
> > > > +	{
> > > > +		.name = "max14577-regulator",
> > > > +		.of_compatible = "maxim,max14577-regulator",
> > > > +	},
> 
> > > Why is there a compatible for this at all, would this ever appear as
> > > part of a different device?
> 
> > For automatic assigning of it's of_node.
> 
> > In previous incarnations of this set it was done manually for some reason.
> 
> The usual thing to do is of_get_child_by_name() on the parent to get the
> container to search in.

What do you mean when you say 'usual thing'? Only the max8998 does
this currently. IMHO the better way to do it for MFD devices is
provide a compatible string and let the framework does the rest for
you.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 1/2] regulator: max14577: Add regulator driver for Maxim 14577
  2013-11-27 16:13       ` Lee Jones
@ 2013-11-27 16:28         ` Mark Brown
  2013-12-02 14:12           ` Krzysztof Kozlowski
  0 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2013-11-27 16:28 UTC (permalink / raw)
  To: Lee Jones
  Cc: Krzysztof Kozlowski, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, Rob Landley, Samuel Ortiz,
	Liam Girdwood, devicetree, linux-doc, linux-kernel, Kyungmin Park,
	Bartlomiej Zolnierkiewicz, Marek Szyprowski

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

On Wed, Nov 27, 2013 at 04:13:23PM +0000, Lee Jones wrote:
> On Wed, 27 Nov 2013, Mark Brown wrote:

> > The usual thing to do is of_get_child_by_name() on the parent to get the
> > container to search in.

> What do you mean when you say 'usual thing'? Only the max8998 does
> this currently. IMHO the better way to do it for MFD devices is
> provide a compatible string and let the framework does the rest for
> you.

No, as3722, Palmas and three of the TPS chips do the same thing.  If it
were a purely Linux-internal thing I'd probably agree with you but as it
is we'd just be adding compatible strings in everyone's DTs to save one
line of code in the kernel.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 1/2] regulator: max14577: Add regulator driver for Maxim 14577
  2013-11-27 16:28         ` Mark Brown
@ 2013-12-02 14:12           ` Krzysztof Kozlowski
  2013-12-03 18:10             ` Mark Brown
  0 siblings, 1 reply; 10+ messages in thread
From: Krzysztof Kozlowski @ 2013-12-02 14:12 UTC (permalink / raw)
  To: Mark Brown
  Cc: Lee Jones, Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, Rob Landley, Samuel Ortiz, Liam Girdwood,
	devicetree, linux-doc, linux-kernel, Kyungmin Park,
	Bartlomiej Zolnierkiewicz, Marek Szyprowski

Hi Mark,

On Wed, 2013-11-27 at 16:28 +0000, Mark Brown wrote:
> On Wed, Nov 27, 2013 at 04:13:23PM +0000, Lee Jones wrote:
> > On Wed, 27 Nov 2013, Mark Brown wrote:
> 
> > > The usual thing to do is of_get_child_by_name() on the parent to get the
> > > container to search in.
> 
> > What do you mean when you say 'usual thing'? Only the max8998 does
> > this currently. IMHO the better way to do it for MFD devices is
> > provide a compatible string and let the framework does the rest for
> > you.
> 
> No, as3722, Palmas and three of the TPS chips do the same thing.  If it
> were a purely Linux-internal thing I'd probably agree with you but as it
> is we'd just be adding compatible strings in everyone's DTs to save one
> line of code in the kernel.

Lee applied the MFD part of max14577, including the "of_compatible" of
max14577-regulator.

Do you wish me to change the patch to previous version using
of_get_child_by_name()?

Best regards,
Krzysztof




^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 1/2] regulator: max14577: Add regulator driver for Maxim 14577
  2013-12-02 14:12           ` Krzysztof Kozlowski
@ 2013-12-03 18:10             ` Mark Brown
  0 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2013-12-03 18:10 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Lee Jones, Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren,
	Ian Campbell, Rob Landley, Samuel Ortiz, Liam Girdwood,
	devicetree, linux-doc, linux-kernel, Kyungmin Park,
	Bartlomiej Zolnierkiewicz, Marek Szyprowski

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

On Mon, Dec 02, 2013 at 03:12:35PM +0100, Krzysztof Kozlowski wrote:

> Do you wish me to change the patch to previous version using
> of_get_child_by_name()?

It'd be more standard, yes.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2013-12-03 18:10 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-27 13:46 [PATCH v5 1/2] regulator: max14577: Add regulator driver for Maxim 14577 Krzysztof Kozlowski
     [not found] ` <1385559982-32039-1-git-send-email-k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2013-11-27 13:46   ` [PATCH v5 2/2] mfd: max14577: Add device tree bindings document Krzysztof Kozlowski
2013-11-27 13:59   ` [PATCH v5 1/2] regulator: max14577: Add regulator driver for Maxim 14577 Lee Jones
2013-11-27 15:20 ` Mark Brown
2013-11-27 15:23   ` Lee Jones
2013-11-27 15:42     ` Mark Brown
2013-11-27 16:13       ` Lee Jones
2013-11-27 16:28         ` Mark Brown
2013-12-02 14:12           ` Krzysztof Kozlowski
2013-12-03 18:10             ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).