Groups > linux.kernel > #1259112 > unrolled thread

[PATCH v5 0/5] mfd: tps65912: Driver rewrite with DT support

Started by	"Andrew F. Davis" <afd@ti.com>
First post	2015-10-30 00:50 +0100
Last post	2015-10-30 20:30 +0100
Articles	7 — 2 participants

Back to article view | Back to linux.kernel

  [PATCH v5 0/5] mfd: tps65912: Driver rewrite with DT support "Andrew F. Davis" <afd@ti.com> - 2015-10-30 00:50 +0100
    [PATCH v5 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC "Andrew F. Davis" <afd@ti.com> - 2015-10-30 00:50 +0100
    [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the TPS65912 PMIC "Andrew F. Davis" <afd@ti.com> - 2015-10-30 00:50 +0100
      Re: [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the  TPS65912 PMIC Lee Jones <lee.jones@linaro.org> - 2015-10-30 18:20 +0100
        Re: [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the  TPS65912 PMIC "Andrew F. Davis" <afd@ti.com> - 2015-10-30 19:10 +0100
          Re: [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the  TPS65912 PMIC Lee Jones <lee.jones@linaro.org> - 2015-10-30 20:10 +0100
            Re: [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the  TPS65912 PMIC "Andrew F. Davis" <afd@ti.com> - 2015-10-30 20:30 +0100

#1259112 — [PATCH v5 0/5] mfd: tps65912: Driver rewrite with DT support

From	"Andrew F. Davis" <afd@ti.com>
Date	2015-10-30 00:50 +0100
Subject	[PATCH v5 0/5] mfd: tps65912: Driver rewrite with DT support
Message-ID	<qp4Xv-6IO-7@gated-at.bofh.it>

In an effort to cleanup this driver and add Device Tree support
the driver has been rewritten based on new driver styles and
modern kernel driver helpers. This has nearly halved the lines
of code while keeping all previous functionality.

Platform file based initialization has been dropped as there is
no examples of this use in the kernel.

v1 can be found here: [1] v2: [2] v3: [3] v4: [4]

Changes from v4:
 - Use mfd core to add sub-devices

Changes from v3:
 - Reorganized regulator driver and related DT node
 - Other small fixes as discussed in v3 thread

Changes from v2:
 - Split the series further into subsystems

Changes from v1:
 - Split the rewrite into delete/create patches
 - Several small fixes as discussed in v1 thread

[1] http://www.spinics.net/lists/devicetree/msg93863.html
[2] http://www.spinics.net/lists/devicetree/msg95003.html
[3] http://www.spinics.net/lists/devicetree/msg95133.html
[4] http://www.spinics.net/lists/devicetree/msg96109.html

Andrew F. Davis (5):
  Documentation: tps65912: Add DT bindings for the TPS65912 PMIC
  mfd: tps65912: Remove old driver in preparation for new driver
  mfd: tps65912: Add driver for the TPS65912 PMIC
  regulator: tps65912: Add regulator driver for the TPS65912 PMIC
  gpio: tps65912: Add GPIO driver for the TPS65912 PMIC

 .../devicetree/bindings/gpio/gpio-tps65912.txt     |  16 +
 Documentation/devicetree/bindings/mfd/tps65912.txt |  51 ++
 .../bindings/regulator/tps65912-regulator.txt      |  28 +
 drivers/gpio/Kconfig                               |   2 +-
 drivers/gpio/gpio-tps65912.c                       | 289 ++++-----
 drivers/mfd/Kconfig                                |  22 +-
 drivers/mfd/Makefile                               |   3 +-
 drivers/mfd/tps65912-core.c                        | 297 ++++-----
 drivers/mfd/tps65912-i2c.c                         | 220 +++----
 drivers/mfd/tps65912-irq.c                         | 217 -------
 drivers/mfd/tps65912-spi.c                         | 221 +++----
 drivers/regulator/Kconfig                          |   2 +-
 drivers/regulator/tps65912-regulator.c             | 711 +++++----------------
 include/linux/mfd/tps65912.h                       | 211 +++---
 14 files changed, 812 insertions(+), 1478 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-tps65912.txt
 create mode 100644 Documentation/devicetree/bindings/mfd/tps65912.txt
 create mode 100644 Documentation/devicetree/bindings/regulator/tps65912-regulator.txt
 rewrite drivers/gpio/gpio-tps65912.c (69%)
 rewrite drivers/mfd/tps65912-core.c (95%)
 rewrite drivers/mfd/tps65912-i2c.c (93%)
 delete mode 100644 drivers/mfd/tps65912-irq.c
 rewrite drivers/mfd/tps65912-spi.c (92%)
 rewrite drivers/regulator/tps65912-regulator.c (94%)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1259114 — [PATCH v5 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

From	"Andrew F. Davis" <afd@ti.com>
Date	2015-10-30 00:50 +0100
Subject	[PATCH v5 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC
Message-ID	<qp57c-6Mj-25@gated-at.bofh.it>
In reply to	#1259112

This patch adds support for TPS65912 PMIC regulators.

The regulators set consists of 4 DCDCs and 10 LDOs. The output
voltages are configurable and are meant to supply power to the
main processor and other components.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 drivers/regulator/Kconfig              |   6 ++
 drivers/regulator/Makefile             |   1 +
 drivers/regulator/tps65912-regulator.c | 170 +++++++++++++++++++++++++++++++++
 3 files changed, 177 insertions(+)
 create mode 100644 drivers/regulator/tps65912-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 3cb2de9..1dec96a 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -727,6 +727,12 @@ config REGULATOR_TPS65910
 	help
 	  This driver supports TPS65910/TPS65911 voltage regulator chips.
 
+config REGULATOR_TPS65912
+	tristate "TI TPS65912 Power regulator"
+	depends on MFD_TPS65912
+	help
+	    This driver supports TPS65912 voltage regulator chip.
+
 config REGULATOR_TPS80031
 	tristate "TI TPS80031/TPS80032 power regualtor driver"
 	depends on MFD_TPS80031
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 222ff5f..0f81749 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -91,6 +91,7 @@ obj-$(CONFIG_REGULATOR_TPS65218) += tps65218-regulator.o
 obj-$(CONFIG_REGULATOR_TPS6524X) += tps6524x-regulator.o
 obj-$(CONFIG_REGULATOR_TPS6586X) += tps6586x-regulator.o
 obj-$(CONFIG_REGULATOR_TPS65910) += tps65910-regulator.o
+obj-$(CONFIG_REGULATOR_TPS65912) += tps65912-regulator.o
 obj-$(CONFIG_REGULATOR_TPS80031) += tps80031-regulator.o
 obj-$(CONFIG_REGULATOR_TWL4030) += twl-regulator.o
 obj-$(CONFIG_REGULATOR_VEXPRESS) += vexpress.o
diff --git a/drivers/regulator/tps65912-regulator.c b/drivers/regulator/tps65912-regulator.c
new file mode 100644
index 0000000..1c9e474
--- /dev/null
+++ b/drivers/regulator/tps65912-regulator.c
@@ -0,0 +1,170 @@
+/*
+ * Regulator driver for TPS65912x PMIC
+ *
+ * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * Author: Andrew F. Davis <afd@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether expressed or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License version 2 for more details.
+ *
+ * Based on the TPS65218 driver and the previous TPS65912 driver by
+ * Margarita Olaya Cabrera <magi@slimlogic.co.uk>
+ */
+
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
+
+#include <linux/mfd/tps65912.h>
+
+enum tps65912_regulators { DCDC1, DCDC2, DCDC3, DCDC4, LDO1, LDO2, LDO3,
+	LDO4, LDO5, LDO6, LDO7, LDO8, LDO9, LDO10 };
+
+#define TPS65912_REGULATOR(_name, _id, _of_match, _ops, _vr, _er, _lr)	\
+	[_id] = {							\
+		.name			= _name,			\
+		.of_match		= of_match_ptr(_of_match),	\
+		.id			= _id,				\
+		.ops			= &_ops,			\
+		.n_voltages		= 64,				\
+		.type			= REGULATOR_VOLTAGE,		\
+		.owner			= THIS_MODULE,			\
+		.vsel_reg		= _vr,				\
+		.vsel_mask		= 0x3f,				\
+		.enable_reg		= _er,				\
+		.enable_mask		= BIT(7),			\
+		.volt_table		= NULL,				\
+		.linear_ranges		= _lr,				\
+		.n_linear_ranges	= ARRAY_SIZE(_lr),		\
+	}
+
+static const struct regulator_linear_range tps65912_dcdc_ranges[] = {
+	REGULATOR_LINEAR_RANGE(500000, 0x0, 0x3f, 50000),
+};
+
+static const struct regulator_linear_range tps65912_ldo_ranges[] = {
+	REGULATOR_LINEAR_RANGE(800000, 0x0, 0x20, 25000),
+	REGULATOR_LINEAR_RANGE(1650000, 0x21, 0x3c, 50000),
+	REGULATOR_LINEAR_RANGE(3100000, 0x3d, 0x3f, 100000),
+};
+
+/* Operations permitted on DCDCx */
+static struct regulator_ops tps65912_ops_dcdc = {
+	.is_enabled		= regulator_is_enabled_regmap,
+	.enable			= regulator_enable_regmap,
+	.disable		= regulator_disable_regmap,
+	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
+	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
+	.list_voltage		= regulator_list_voltage_linear_range,
+};
+
+/* Operations permitted on LDOx */
+static struct regulator_ops tps65912_ops_ldo = {
+	.is_enabled		= regulator_is_enabled_regmap,
+	.enable			= regulator_enable_regmap,
+	.disable		= regulator_disable_regmap,
+	.get_voltage_sel	= regulator_get_voltage_sel_regmap,
+	.set_voltage_sel	= regulator_set_voltage_sel_regmap,
+	.list_voltage		= regulator_list_voltage_linear_range,
+	.map_voltage		= regulator_map_voltage_linear_range,
+};
+
+static const struct regulator_desc regulators[] = {
+	TPS65912_REGULATOR("DCDC1", DCDC1, "dcdc1", tps65912_ops_dcdc,
+			   TPS65912_DCDC1_OP, TPS65912_DCDC1_CTRL,
+			   tps65912_dcdc_ranges),
+	TPS65912_REGULATOR("DCDC2", DCDC2, "dcdc2", tps65912_ops_dcdc,
+			   TPS65912_DCDC2_OP, TPS65912_DCDC2_CTRL,
+			   tps65912_dcdc_ranges),
+	TPS65912_REGULATOR("DCDC3", DCDC3, "dcdc3", tps65912_ops_dcdc,
+			   TPS65912_DCDC3_OP, TPS65912_DCDC3_CTRL,
+			   tps65912_dcdc_ranges),
+	TPS65912_REGULATOR("DCDC4", DCDC4, "dcdc4", tps65912_ops_dcdc,
+			   TPS65912_DCDC4_OP, TPS65912_DCDC4_CTRL,
+			   tps65912_dcdc_ranges),
+	TPS65912_REGULATOR("LDO1", LDO1, "ldo1", tps65912_ops_ldo,
+			   TPS65912_LDO1_OP, TPS65912_LDO1_AVS,
+			   tps65912_ldo_ranges),
+	TPS65912_REGULATOR("LDO2", LDO2, "ldo2", tps65912_ops_ldo,
+			   TPS65912_LDO2_OP, TPS65912_LDO2_AVS,
+			   tps65912_ldo_ranges),
+	TPS65912_REGULATOR("LDO3", LDO3, "ldo3", tps65912_ops_ldo,
+			   TPS65912_LDO3_OP, TPS65912_LDO3_AVS,
+			   tps65912_ldo_ranges),
+	TPS65912_REGULATOR("LDO4", LDO4, "ldo4", tps65912_ops_ldo,
+			   TPS65912_LDO4_OP, TPS65912_LDO4_AVS,
+			   tps65912_ldo_ranges),
+	TPS65912_REGULATOR("LDO5", LDO5, "ldo5", tps65912_ops_ldo,
+			   TPS65912_LDO5, TPS65912_LDO5,
+			   tps65912_ldo_ranges),
+	TPS65912_REGULATOR("LDO6", LDO6, "ldo6", tps65912_ops_ldo,
+			   TPS65912_LDO6, TPS65912_LDO6,
+			   tps65912_ldo_ranges),
+	TPS65912_REGULATOR("LDO7", LDO7, "ldo7", tps65912_ops_ldo,
+			   TPS65912_LDO7, TPS65912_LDO7,
+			   tps65912_ldo_ranges),
+	TPS65912_REGULATOR("LDO8", LDO8, "ldo8", tps65912_ops_ldo,
+			   TPS65912_LDO8, TPS65912_LDO8,
+			   tps65912_ldo_ranges),
+	TPS65912_REGULATOR("LDO9", LDO9, "ldo9", tps65912_ops_ldo,
+			   TPS65912_LDO9, TPS65912_LDO9,
+			   tps65912_ldo_ranges),
+	TPS65912_REGULATOR("LDO10", LDO10, "ldo10", tps65912_ops_ldo,
+			   TPS65912_LDO10, TPS65912_LDO10,
+			   tps65912_ldo_ranges),
+};
+
+static int tps65912_regulator_probe(struct platform_device *pdev)
+{
+	struct tps65912 *tps = dev_get_drvdata(pdev->dev.parent);
+	struct regulator_config config = { };
+	struct regulator_dev *rdev;
+	int i;
+
+	platform_set_drvdata(pdev, tps);
+
+	config.dev = &pdev->dev;
+	config.driver_data = tps;
+	config.of_node = pdev->dev.of_node;
+	config.regmap = tps->regmap;
+
+	for (i = 0; i < ARRAY_SIZE(regulators); i++) {
+		rdev = devm_regulator_register(&pdev->dev, &regulators[i],
+					       &config);
+		if (IS_ERR(rdev)) {
+			dev_err(tps->dev, "failed to register %s regulator\n",
+				pdev->name);
+			return PTR_ERR(rdev);
+		}
+	}
+
+	return 0;
+}
+
+static const struct platform_device_id tps65912_regulator_id_table[] = {
+	{ "tps65912-regulator", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, tps65912_regulator_id_table);
+
+static struct platform_driver tps65912_regulator_driver = {
+	.driver = {
+		.name = "tps65912-regulator",
+	},
+	.probe = tps65912_regulator_probe,
+	.id_table = tps65912_regulator_id_table,
+};
+module_platform_driver(tps65912_regulator_driver);
+
+MODULE_AUTHOR("Andrew F. Davis <afd@ti.com>");
+MODULE_DESCRIPTION("TPS65912 voltage regulator driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1259126 — [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the TPS65912 PMIC

From	"Andrew F. Davis" <afd@ti.com>
Date	2015-10-30 00:50 +0100
Subject	[PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the TPS65912 PMIC
Message-ID	<qp57d-6Mj-45@gated-at.bofh.it>
In reply to	#1259112

The TPS65912 PMIC contains several regulators and a GPIO controller.
Add bindings for the TPS65912 PMIC.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 .../devicetree/bindings/gpio/gpio-tps65912.txt     | 16 +++++++
 Documentation/devicetree/bindings/mfd/tps65912.txt | 51 ++++++++++++++++++++++
 .../bindings/regulator/tps65912-regulator.txt      | 28 ++++++++++++
 3 files changed, 95 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-tps65912.txt
 create mode 100644 Documentation/devicetree/bindings/mfd/tps65912.txt
 create mode 100644 Documentation/devicetree/bindings/regulator/tps65912-regulator.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-tps65912.txt b/Documentation/devicetree/bindings/gpio/gpio-tps65912.txt
new file mode 100644
index 0000000..0c5c05c4
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-tps65912.txt
@@ -0,0 +1,16 @@
+* TPS65912 GPIO Controller bindings
+
+Required properties:
+ - compatible : Should be "ti,tps65912-gpio".
+ - gpio-controller : Marks the device node as a GPIO Controller.
+ - #gpio-cells : Should be two.  The first cell is the pin number and
+     the second cell is used to specify flags.
+     See include/dt-bindings/gpio/gpio.h for possible values.
+
+Example:
+
+	gpio4: tps65912_gpio {
+		compatible = "ti,tps65912-gpio";
+		gpio-controller;
+		#gpio-cells = <2>;
+	};
diff --git a/Documentation/devicetree/bindings/mfd/tps65912.txt b/Documentation/devicetree/bindings/mfd/tps65912.txt
new file mode 100644
index 0000000..da5804a
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/tps65912.txt
@@ -0,0 +1,51 @@
+* TPS65912 Power Management Integrated Circuit bindings
+
+Required properties:
+ - compatible : Should be "ti,tps65912".
+ - reg : Slave address or chip select number (I2C / SPI).
+ - interrupt-parent : The parent interrupt controller.
+ - interrupts : The interrupt line the device is connected to.
+ - interrupt-controller : Marks the device node as an interrupt controller.
+ - #interrupt-cells: The number of cells to describe an IRQ, this should be 2.
+     The first cell is the IRQ number.
+     The second cell is the flags, encoded as the trigger masks from
+     ../interrupt-controller/interrupts.txt
+
+Additional nodes defined in:
+ - Regulators: ../regulator/tps65912-regulator.txt
+ - GPIO: ../gpio/gpio-tps65912.txt.
+
+Example:
+
+	pmic: tps65912@2d {
+		compatible = "ti,tps65912";
+		reg = <0x2d>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <28 IRQ_TYPE_LEVEL_LOW>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+
+		regulators {
+			compatible = "ti,tps65912-regulator";
+
+			dcdc1 {
+				regulator-name = "vdd_core";
+				regulator-min-microvolt = <912000>;
+				regulator-max-microvolt = <1144000>;
+				regulator-boot-on;
+				regulator-always-on;
+			};
+
+			ldo1 {
+				regulator-name = "ldo1";
+				regulator-min-microvolt = <1900000>;
+				regulator-max-microvolt = <1900000>;
+			};
+		};
+
+		gpio4: tps65912_gpio {
+			compatible = "ti,tps65912-gpio";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+	};
diff --git a/Documentation/devicetree/bindings/regulator/tps65912-regulator.txt b/Documentation/devicetree/bindings/regulator/tps65912-regulator.txt
new file mode 100644
index 0000000..4cd05fa
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/tps65912-regulator.txt
@@ -0,0 +1,28 @@
+* TPS65912 regulator bindings
+
+Required properties:
+ - compatible: "ti,tps65912-regulator"
+ - list of regulators provided by this controller, must be named
+     after their hardware counterparts: dcdc[1-4] and ldo[1-10]
+
+Each regulator is defined using the standard binding for regulators.
+
+Example:
+
+	tps65912_regulator {
+		compatible = "ti,tps65912-regulator";
+
+		dcdc1 {
+			regulator-name = "vdd_core";
+			regulator-min-microvolt = <912000>;
+			regulator-max-microvolt = <1144000>;
+			regulator-boot-on;
+			regulator-always-on;
+		};
+
+		ldo1 {
+			regulator-name = "ldo1";
+			regulator-min-microvolt = <1900000>;
+			regulator-max-microvolt = <1900000>;
+		};
+	};
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1259645 — Re: [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the TPS65912 PMIC

From	Lee Jones <lee.jones@linaro.org>
Date	2015-10-30 18:20 +0100
Subject	Re: [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the TPS65912 PMIC
Message-ID	<qplvl-8A-25@gated-at.bofh.it>
In reply to	#1259126

On Thu, 29 Oct 2015, Andrew F. Davis wrote:

> The TPS65912 PMIC contains several regulators and a GPIO controller.
> Add bindings for the TPS65912 PMIC.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
> ---
>  .../devicetree/bindings/gpio/gpio-tps65912.txt     | 16 +++++++

Why have you dropped Linus' Review-by?

>  Documentation/devicetree/bindings/mfd/tps65912.txt | 51 ++++++++++++++++++++++
>  .../bindings/regulator/tps65912-regulator.txt      | 28 ++++++++++++
>  3 files changed, 95 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-tps65912.txt
>  create mode 100644 Documentation/devicetree/bindings/mfd/tps65912.txt
>  create mode 100644 Documentation/devicetree/bindings/regulator/tps65912-regulator.txt
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-tps65912.txt b/Documentation/devicetree/bindings/gpio/gpio-tps65912.txt
> new file mode 100644
> index 0000000..0c5c05c4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-tps65912.txt
> @@ -0,0 +1,16 @@
> +* TPS65912 GPIO Controller bindings
> +
> +Required properties:
> + - compatible : Should be "ti,tps65912-gpio".
> + - gpio-controller : Marks the device node as a GPIO Controller.
> + - #gpio-cells : Should be two.  The first cell is the pin number and

You should add a (See: ../gpio/gpio.txt) here I think.

> +     the second cell is used to specify flags.
> +     See include/dt-bindings/gpio/gpio.h for possible values.

This is a Linuxisum and shouldn't really live in here.

I think it would be better to document them in ../gpio/gpio.txt and
reference that instead.

> +Example:
> +
> +	gpio4: tps65912_gpio {

Device names don't belong in node names.

Just "gpio" will do.

> +		compatible = "ti,tps65912-gpio";
> +		gpio-controller;
> +		#gpio-cells = <2>;
> +	};
> diff --git a/Documentation/devicetree/bindings/mfd/tps65912.txt b/Documentation/devicetree/bindings/mfd/tps65912.txt
> new file mode 100644
> index 0000000..da5804a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/tps65912.txt
> @@ -0,0 +1,51 @@
> +* TPS65912 Power Management Integrated Circuit bindings
> +
> +Required properties:
> + - compatible : Should be "ti,tps65912".
> + - reg : Slave address or chip select number (I2C / SPI).
> + - interrupt-parent : The parent interrupt controller.
> + - interrupts : The interrupt line the device is connected to.
> + - interrupt-controller : Marks the device node as an interrupt controller.
> + - #interrupt-cells: The number of cells to describe an IRQ, this should be 2.
> +     The first cell is the IRQ number.
> +     The second cell is the flags, encoded as the trigger masks from
> +     ../interrupt-controller/interrupts.txt

Nit: We *normally* treat these as bullet-points and not place
full-stops on them:

$ git grep "compatible" -- Documentation/devicetree/bindings/ | grep -v "\.$" | wc -l
5227
$ git grep "compatible.*\.$" -- Documentation/devicetree/bindings/ | wc -l
486

... and I *personally* think it's easier to read the document if it's
formatted as follows:

- compatible			: Should be "ti,tps65912".
- reg				: Slave address or chip select number (I2C / SPI).
- interrupt-parent		: The parent interrupt controller.
- interrupts			: The interrupt line the device is connected to.
- interrupt-controller		: Marks the device node as an interrupt controller.
- #interrupt-cells		: The number of cells to describe an IRQ, this should be 2.
					The first cell is the IRQ number.
					The second cell is the flags, encoded as the trigger masks from
					../interrupt-controller/interrupts.txt

... I hope you agree.

[...]

-- 
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 linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1259681 — Re: [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the TPS65912 PMIC

From	"Andrew F. Davis" <afd@ti.com>
Date	2015-10-30 19:10 +0100
Subject	Re: [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the TPS65912 PMIC
Message-ID	<qpmhJ-Ey-23@gated-at.bofh.it>
In reply to	#1259645

On 10/30/2015 12:10 PM, Lee Jones wrote:
> On Thu, 29 Oct 2015, Andrew F. Davis wrote:
>
>> The TPS65912 PMIC contains several regulators and a GPIO controller.
>> Add bindings for the TPS65912 PMIC.
>>
>> Signed-off-by: Andrew F. Davis <afd@ti.com>
>> ---
>>   .../devicetree/bindings/gpio/gpio-tps65912.txt     | 16 +++++++
>
> Why have you dropped Linus' Review-by?
>

Strange, I thought I made a change to this. Well this brings up a question,
how much change can we have before we are supposed to drop Reviewed/Acked-by?

>>   Documentation/devicetree/bindings/mfd/tps65912.txt | 51 ++++++++++++++++++++++
>>   .../bindings/regulator/tps65912-regulator.txt      | 28 ++++++++++++
>>   3 files changed, 95 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/gpio/gpio-tps65912.txt
>>   create mode 100644 Documentation/devicetree/bindings/mfd/tps65912.txt
>>   create mode 100644 Documentation/devicetree/bindings/regulator/tps65912-regulator.txt
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-tps65912.txt b/Documentation/devicetree/bindings/gpio/gpio-tps65912.txt
>> new file mode 100644
>> index 0000000..0c5c05c4
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpio/gpio-tps65912.txt
>> @@ -0,0 +1,16 @@
>> +* TPS65912 GPIO Controller bindings
>> +
>> +Required properties:
>> + - compatible : Should be "ti,tps65912-gpio".
>> + - gpio-controller : Marks the device node as a GPIO Controller.
>> + - #gpio-cells : Should be two.  The first cell is the pin number and
>
> You should add a (See: ../gpio/gpio.txt) here I think.
>

ACK

>> +     the second cell is used to specify flags.
>> +     See include/dt-bindings/gpio/gpio.h for possible values.
>
> This is a Linuxisum and shouldn't really live in here.
>
> I think it would be better to document them in ../gpio/gpio.txt and
> reference that instead.
>

Looks like that is already in ../gpio/gpio.txt:57

>> +Example:
>> +
>> +	gpio4: tps65912_gpio {
>
> Device names don't belong in node names.
>
> Just "gpio" will do.
>

ACK

>> +		compatible = "ti,tps65912-gpio";
>> +		gpio-controller;
>> +		#gpio-cells = <2>;
>> +	};
>> diff --git a/Documentation/devicetree/bindings/mfd/tps65912.txt b/Documentation/devicetree/bindings/mfd/tps65912.txt
>> new file mode 100644
>> index 0000000..da5804a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/tps65912.txt
>> @@ -0,0 +1,51 @@
>> +* TPS65912 Power Management Integrated Circuit bindings
>> +
>> +Required properties:
>> + - compatible : Should be "ti,tps65912".
>> + - reg : Slave address or chip select number (I2C / SPI).
>> + - interrupt-parent : The parent interrupt controller.
>> + - interrupts : The interrupt line the device is connected to.
>> + - interrupt-controller : Marks the device node as an interrupt controller.
>> + - #interrupt-cells: The number of cells to describe an IRQ, this should be 2.
>> +     The first cell is the IRQ number.
>> +     The second cell is the flags, encoded as the trigger masks from
>> +     ../interrupt-controller/interrupts.txt
>
> Nit: We *normally* treat these as bullet-points and not place
> full-stops on them:
>
> $ git grep "compatible" -- Documentation/devicetree/bindings/ | grep -v "\.$" | wc -l
> 5227
> $ git grep "compatible.*\.$" -- Documentation/devicetree/bindings/ | wc -l
> 486
>

What about for multi-sentence descriptions, we need the middle full-stops, then to not
have one on the end seems kinda odd looking.

> ... and I *personally* think it's easier to read the document if it's
> formatted as follows:
>
> - compatible			: Should be "ti,tps65912".
> - reg				: Slave address or chip select number (I2C / SPI).
> - interrupt-parent		: The parent interrupt controller.
> - interrupts			: The interrupt line the device is connected to.
> - interrupt-controller		: Marks the device node as an interrupt controller.
> - #interrupt-cells		: The number of cells to describe an IRQ, this should be 2.
> 					The first cell is the IRQ number.
> 					The second cell is the flags, encoded as the trigger masks from
> 					../interrupt-controller/interrupts.txt
>
> ... I hope you agree.
>

Yeah, I did that with the last driver, not sure why I forgot to fix that here, I'll fix this.

> [...]
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1259726 — Re: [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the TPS65912 PMIC

From	Lee Jones <lee.jones@linaro.org>
Date	2015-10-30 20:10 +0100
Subject	Re: [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the TPS65912 PMIC
Message-ID	<qpndL-1fI-3@gated-at.bofh.it>
In reply to	#1259681

On Fri, 30 Oct 2015, Andrew F. Davis wrote:

> On 10/30/2015 12:10 PM, Lee Jones wrote:
> >On Thu, 29 Oct 2015, Andrew F. Davis wrote:
> >
> >>The TPS65912 PMIC contains several regulators and a GPIO controller.
> >>Add bindings for the TPS65912 PMIC.
> >>
> >>Signed-off-by: Andrew F. Davis <afd@ti.com>
> >>---
> >>  .../devicetree/bindings/gpio/gpio-tps65912.txt     | 16 +++++++
> >
> >Why have you dropped Linus' Review-by?
> >
> 
> Strange, I thought I made a change to this. Well this brings up a question,
> how much change can we have before we are supposed to drop Reviewed/Acked-by?

Common sense call I'm afraid. ;)

[...]

> >>+     the second cell is used to specify flags.
> >>+     See include/dt-bindings/gpio/gpio.h for possible values.
> >
> >This is a Linuxisum and shouldn't really live in here.
> >
> >I think it would be better to document them in ../gpio/gpio.txt and
> >reference that instead.
> >
> 
> Looks like that is already in ../gpio/gpio.txt:57

There is a mention of GPIO_ACTIVE_HIGH, as it's used in an example.
However GPIO_ACTIVE_LOW is missing.  I think both could do with
documenting properly, then you can refer to them from here.

[...]

> >>+Required properties:
> >>+ - compatible : Should be "ti,tps65912".
> >>+ - reg : Slave address or chip select number (I2C / SPI).
> >>+ - interrupt-parent : The parent interrupt controller.
> >>+ - interrupts : The interrupt line the device is connected to.
> >>+ - interrupt-controller : Marks the device node as an interrupt controller.
> >>+ - #interrupt-cells: The number of cells to describe an IRQ, this should be 2.
> >>+     The first cell is the IRQ number.
> >>+     The second cell is the flags, encoded as the trigger masks from
> >>+     ../interrupt-controller/interrupts.txt
> >
> >Nit: We *normally* treat these as bullet-points and not place
> >full-stops on them:
> >
> >$ git grep "compatible" -- Documentation/devicetree/bindings/ | grep -v "\.$" | wc -l
> >5227
> >$ git grep "compatible.*\.$" -- Documentation/devicetree/bindings/ | wc -l
> >486
> >
> 
> What about for multi-sentence descriptions, we need the middle full-stops, then to not
> have one on the end seems kinda odd looking.

That's the way I usually do it -- doesn't look too bad. ;)

[...]

-- 
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 linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1259742 — Re: [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the TPS65912 PMIC

From	"Andrew F. Davis" <afd@ti.com>
Date	2015-10-30 20:30 +0100
Subject	Re: [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the TPS65912 PMIC
Message-ID	<qpnx8-1mb-19@gated-at.bofh.it>
In reply to	#1259726

On 10/30/2015 02:06 PM, Lee Jones wrote:
> On Fri, 30 Oct 2015, Andrew F. Davis wrote:
>
>> On 10/30/2015 12:10 PM, Lee Jones wrote:
>>> On Thu, 29 Oct 2015, Andrew F. Davis wrote:
>>>
>>>> The TPS65912 PMIC contains several regulators and a GPIO controller.
>>>> Add bindings for the TPS65912 PMIC.
>>>>
>>>> Signed-off-by: Andrew F. Davis <afd@ti.com>
>>>> ---
>>>>   .../devicetree/bindings/gpio/gpio-tps65912.txt     | 16 +++++++
>>>
>>> Why have you dropped Linus' Review-by?
>>>
>>
>> Strange, I thought I made a change to this. Well this brings up a question,
>> how much change can we have before we are supposed to drop Reviewed/Acked-by?
>
> Common sense call I'm afraid. ;)
>
> [...]
>
>>>> +     the second cell is used to specify flags.
>>>> +     See include/dt-bindings/gpio/gpio.h for possible values.
>>>
>>> This is a Linuxisum and shouldn't really live in here.
>>>
>>> I think it would be better to document them in ../gpio/gpio.txt and
>>> reference that instead.
>>>
>>
>> Looks like that is already in ../gpio/gpio.txt:57
>
> There is a mention of GPIO_ACTIVE_HIGH, as it's used in an example.
> However GPIO_ACTIVE_LOW is missing.  I think both could do with
> documenting properly, then you can refer to them from here.
>

I mean the lines above the example, they say to use the macros
defined in include/dt-bindings/gpio/gpio.h whenever possible,
that's really all I would say.

> [...]
>
>>>> +Required properties:
>>>> + - compatible : Should be "ti,tps65912".
>>>> + - reg : Slave address or chip select number (I2C / SPI).
>>>> + - interrupt-parent : The parent interrupt controller.
>>>> + - interrupts : The interrupt line the device is connected to.
>>>> + - interrupt-controller : Marks the device node as an interrupt controller.
>>>> + - #interrupt-cells: The number of cells to describe an IRQ, this should be 2.
>>>> +     The first cell is the IRQ number.
>>>> +     The second cell is the flags, encoded as the trigger masks from
>>>> +     ../interrupt-controller/interrupts.txt
>>>
>>> Nit: We *normally* treat these as bullet-points and not place
>>> full-stops on them:
>>>
>>> $ git grep "compatible" -- Documentation/devicetree/bindings/ | grep -v "\.$" | wc -l
>>> 5227
>>> $ git grep "compatible.*\.$" -- Documentation/devicetree/bindings/ | wc -l
>>> 486
>>>
>>
>> What about for multi-sentence descriptions, we need the middle full-stops, then to not
>> have one on the end seems kinda odd looking.
>
> That's the way I usually do it -- doesn't look too bad. ;)
>
> [...]
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

csiph-web

[PATCH v5 0/5] mfd: tps65912: Driver rewrite with DT support

Contents

#1259112 — [PATCH v5 0/5] mfd: tps65912: Driver rewrite with DT support

#1259114 — [PATCH v5 4/5] regulator: tps65912: Add regulator driver for the TPS65912 PMIC

#1259126 — [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the TPS65912 PMIC

#1259645 — Re: [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the TPS65912 PMIC

#1259681 — Re: [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the TPS65912 PMIC

#1259726 — Re: [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the TPS65912 PMIC

#1259742 — Re: [PATCH v5 1/5] Documentation: tps65912: Add DT bindings for the TPS65912 PMIC