[PATCH v2 0/4] tps6105x add devicetree and leds support

[PATCH v2 0/4] tps6105x add devicetree and leds support

[Sven Van Asbroeck]

v1 -> v2:
	Select chip operational mode by looking at subnode name, _not_ its
	compatible property. Suggested by Mark Brown.

I needed led operation for this mfd chip, so I added a very simple
driver for this.

My platform (arm imx6q) is devicetree-based, so I added optional
devicetree support for this chip and its sub-drivers.

Interestingly, the main mfd driver had a dt 'compatible' binding, but
could not operate without platform data?

Sven Van Asbroeck (4):
  tps6105x: add optional devicetree support
  regulator: tps6105x: add optional devicetree support
  leds: tps6105x: add driver for mfd chip led mode
  dt-bindings: mfd: update TI tps6105x chip bindings

 .../devicetree/bindings/mfd/tps6105x.txt      | 42 ++++++++-
 drivers/leds/Kconfig                          | 10 +++
 drivers/leds/Makefile                         |  1 +
 drivers/leds/leds-tps6105x.c                  | 87 +++++++++++++++++++
 drivers/mfd/tps6105x.c                        | 34 +++++++-
 drivers/regulator/tps6105x-regulator.c        |  2 +
 include/linux/mfd/tps6105x.h                  |  1 +
 7 files changed, 173 insertions(+), 4 deletions(-)
 create mode 100644 drivers/leds/leds-tps6105x.c

-- 
2.17.1

[PATCH v2 1/4] tps6105x: add optional devicetree support

[Sven Van Asbroeck]

This driver currently requires platform data to specify the
operational mode and regulator init data (in case of regulator
mode).

Optionally specify the operational mode by looking at the name
of the devicetree child node.

Example: put chip in regulator mode:

i2c0 {
	tps61052@33 {
		compatible = "ti,tps61052";
		reg = <0x33>;

		regulator {
                            regulator-min-microvolt = <5000000>;
                            regulator-max-microvolt = <5000000>;
                            regulator-always-on;
		};
	};
};

Tree: linux-next
Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
---
 drivers/mfd/tps6105x.c | 34 +++++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/tps6105x.c b/drivers/mfd/tps6105x.c
index 6ac3607a79c2..c906324d293e 100644
--- a/drivers/mfd/tps6105x.c
+++ b/drivers/mfd/tps6105x.c
@@ -91,6 +91,32 @@ static int tps6105x_add_device(struct tps6105x *tps6105x,
 			       PLATFORM_DEVID_AUTO, cell, 1, NULL, 0, NULL);
 }
 
+static struct tps6105x_platform_data *tps6105x_parse_dt(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct tps6105x_platform_data *pdata;
+	struct device_node *child;
+
+	if (!np)
+		return ERR_PTR(-EINVAL);
+	if (of_get_available_child_count(np) > 1) {
+		dev_err(dev, "cannot support multiple operational modes");
+		return ERR_PTR(-EINVAL);
+	}
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+	pdata->mode = TPS6105X_MODE_SHUTDOWN;
+	for_each_available_child_of_node(np, child) {
+		if (child->name && !of_node_cmp(child->name, "regulator"))
+			pdata->mode = TPS6105X_MODE_VOLTAGE;
+		else if (child->name && !of_node_cmp(child->name, "led"))
+			pdata->mode = TPS6105X_MODE_TORCH;
+	}
+
+	return pdata;
+}
+
 static int tps6105x_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
@@ -99,9 +125,11 @@ static int tps6105x_probe(struct i2c_client *client,
 	int ret;
 
 	pdata = dev_get_platdata(&client->dev);
-	if (!pdata) {
-		dev_err(&client->dev, "missing platform data\n");
-		return -ENODEV;
+	if (!pdata)
+		pdata = tps6105x_parse_dt(&client->dev);
+	if (IS_ERR(pdata)) {
+		dev_err(&client->dev, "No platform data or DT found");
+		return PTR_ERR(pdata);
 	}
 
 	tps6105x = devm_kmalloc(&client->dev, sizeof(*tps6105x), GFP_KERNEL);
-- 
2.17.1

Applied "tps6105x: add optional devicetree support" to the regulator tree

[Mark Brown]

The patch

   tps6105x: add optional devicetree support

has been applied to the regulator tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-5.5

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From 62f7f3eca4c30064ab37b42d97cef4292d75fdd0 Mon Sep 17 00:00:00 2001
From: Sven Van Asbroeck <thesven73@gmail.com>
Date: Tue, 19 Nov 2019 10:46:08 -0500
Subject: [PATCH] tps6105x: add optional devicetree support

This driver currently requires platform data to specify the
operational mode and regulator init data (in case of regulator
mode).

Optionally specify the operational mode by looking at the name
of the devicetree child node.

Example: put chip in regulator mode:

i2c0 {
	tps61052@33 {
		compatible = "ti,tps61052";
		reg = <0x33>;

		regulator {
                            regulator-min-microvolt = <5000000>;
                            regulator-max-microvolt = <5000000>;
                            regulator-always-on;
		};
	};
};

Tree: linux-next
Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
Link: https://lore.kernel.org/r/20191119154611.29625-2-TheSven73@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/mfd/tps6105x.c | 34 +++++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/tps6105x.c b/drivers/mfd/tps6105x.c
index 6ac3607a79c2..c906324d293e 100644
--- a/drivers/mfd/tps6105x.c
+++ b/drivers/mfd/tps6105x.c
@@ -91,6 +91,32 @@ static int tps6105x_add_device(struct tps6105x *tps6105x,
 			       PLATFORM_DEVID_AUTO, cell, 1, NULL, 0, NULL);
 }
 
+static struct tps6105x_platform_data *tps6105x_parse_dt(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct tps6105x_platform_data *pdata;
+	struct device_node *child;
+
+	if (!np)
+		return ERR_PTR(-EINVAL);
+	if (of_get_available_child_count(np) > 1) {
+		dev_err(dev, "cannot support multiple operational modes");
+		return ERR_PTR(-EINVAL);
+	}
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+	pdata->mode = TPS6105X_MODE_SHUTDOWN;
+	for_each_available_child_of_node(np, child) {
+		if (child->name && !of_node_cmp(child->name, "regulator"))
+			pdata->mode = TPS6105X_MODE_VOLTAGE;
+		else if (child->name && !of_node_cmp(child->name, "led"))
+			pdata->mode = TPS6105X_MODE_TORCH;
+	}
+
+	return pdata;
+}
+
 static int tps6105x_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
@@ -99,9 +125,11 @@ static int tps6105x_probe(struct i2c_client *client,
 	int ret;
 
 	pdata = dev_get_platdata(&client->dev);
-	if (!pdata) {
-		dev_err(&client->dev, "missing platform data\n");
-		return -ENODEV;
+	if (!pdata)
+		pdata = tps6105x_parse_dt(&client->dev);
+	if (IS_ERR(pdata)) {
+		dev_err(&client->dev, "No platform data or DT found");
+		return PTR_ERR(pdata);
 	}
 
 	tps6105x = devm_kmalloc(&client->dev, sizeof(*tps6105x), GFP_KERNEL);
-- 
2.20.1

Re: Applied "tps6105x: add optional devicetree support" to the regulator tree

[Lee Jones]

On Wed, 20 Nov 2019, Mark Brown wrote:

> The patch
> 
>    tps6105x: add optional devicetree support
> 
> has been applied to the regulator tree at
> 
>    https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-5.5
> 
> All being well this means that it will be integrated into the linux-next
> tree (usually sometime in the next 24 hours) and sent to Linus during
> the next merge window (or sooner if it is a bug fix), however if
> problems are discovered then the patch may be dropped or reverted.  
> 
> You may get further e-mails resulting from automated or manual testing
> and review of the tree, please engage with people reporting problems and
> send followup patches addressing any issues that are reported if needed.
> 
> If any updates are required or you are submitting further changes they
> should be sent as incremental updates against current git, existing
> patches will not be replaced.
> 
> Please add any relevant lists and maintainers to the CCs when replying
> to this mail.
> 
> Thanks,
> Mark
> 
> From 62f7f3eca4c30064ab37b42d97cef4292d75fdd0 Mon Sep 17 00:00:00 2001
> From: Sven Van Asbroeck <thesven73@gmail.com>
> Date: Tue, 19 Nov 2019 10:46:08 -0500
> Subject: [PATCH] tps6105x: add optional devicetree support
> 
> This driver currently requires platform data to specify the
> operational mode and regulator init data (in case of regulator
> mode).
> 
> Optionally specify the operational mode by looking at the name
> of the devicetree child node.
> 
> Example: put chip in regulator mode:
> 
> i2c0 {
> 	tps61052@33 {
> 		compatible = "ti,tps61052";
> 		reg = <0x33>;
> 
> 		regulator {
>                             regulator-min-microvolt = <5000000>;
>                             regulator-max-microvolt = <5000000>;
>                             regulator-always-on;
> 		};
> 	};
> };
> 
> Tree: linux-next
> Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
> Link: https://lore.kernel.org/r/20191119154611.29625-2-TheSven73@gmail.com
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  drivers/mfd/tps6105x.c | 34 +++++++++++++++++++++++++++++++---
>  1 file changed, 31 insertions(+), 3 deletions(-)

?

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: Applied "tps6105x: add optional devicetree support" to the regulator tree

[Mark Brown]

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

On Fri, Nov 22, 2019 at 07:31:24AM +0000, Lee Jones wrote:
> On Wed, 20 Nov 2019, Mark Brown wrote:

> > Example: put chip in regulator mode:

> > i2c0 {
> > 	tps61052@33 {
> > 		compatible = "ti,tps61052";
> > 		reg = <0x33>;
> > 
> > 		regulator {
> >                             regulator-min-microvolt = <5000000>;
> >                             regulator-max-microvolt = <5000000>;
> >                             regulator-always-on;
> > 		};
> > 	};
> > };

> ?

Sorry, I completely missed that this was adding a MFD file - the binding
only mentioned regulator stuff and I clearly didn't look at the
filename.  Do you want me to drop it?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: Applied "tps6105x: add optional devicetree support" to the regulator tree

[Sven Van Asbroeck]

On Fri, Nov 22, 2019 at 8:32 AM Mark Brown <broonie@kernel.org> wrote:
>
> Sorry, I completely missed that this was adding a MFD file - the binding
> only mentioned regulator stuff and I clearly didn't look at the
> filename.  Do you want me to drop it?

I didn't put "mfd:" on the patch title line, adding to the confusion, sorry.
I'll roll that into the next patch version. Except if the patch is acceptable
to Lee as-is.

Re: Applied "tps6105x: add optional devicetree support" to the regulator tree

[Lee Jones]

On Fri, 22 Nov 2019, Mark Brown wrote:

> On Fri, Nov 22, 2019 at 07:31:24AM +0000, Lee Jones wrote:
> > On Wed, 20 Nov 2019, Mark Brown wrote:
> 
> > > Example: put chip in regulator mode:
> 
> > > i2c0 {
> > > 	tps61052@33 {
> > > 		compatible = "ti,tps61052";
> > > 		reg = <0x33>;
> > > 
> > > 		regulator {
> > >                             regulator-min-microvolt = <5000000>;
> > >                             regulator-max-microvolt = <5000000>;
> > >                             regulator-always-on;
> > > 		};
> > > 	};
> > > };
> 
> > ?
> 
> Sorry, I completely missed that this was adding a MFD file - the binding
> only mentioned regulator stuff and I clearly didn't look at the
> filename.  Do you want me to drop it?

It's okay. Probably won't cause too much chaos. Keep it applied.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v2 2/4] regulator: tps6105x: add optional devicetree support

[Sven Van Asbroeck]

Tell the regulator framework to retrieve regulator init
data from the 'regulator' subnode, or from the parent mfd
device's platform data.

Example:

i2c0 {
	tps61052@33 {
		compatible = "ti,tps61052";
		reg = <0x33>;

		regulator {
			regulator-min-microvolt = <5000000>;
			regulator-max-microvolt = <5000000>;
			regulator-always-on;
		};
	};
};

Tree: next-20191118
Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
---
 drivers/regulator/tps6105x-regulator.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/regulator/tps6105x-regulator.c b/drivers/regulator/tps6105x-regulator.c
index 06059a94f7c6..f8939af0bd2c 100644
--- a/drivers/regulator/tps6105x-regulator.c
+++ b/drivers/regulator/tps6105x-regulator.c
@@ -37,6 +37,7 @@ static struct regulator_ops tps6105x_regulator_ops = {
 
 static const struct regulator_desc tps6105x_regulator_desc = {
 	.name		= "tps6105x-boost",
+	.of_match	= of_match_ptr("regulator"),
 	.ops		= &tps6105x_regulator_ops,
 	.type		= REGULATOR_VOLTAGE,
 	.id		= 0,
@@ -71,6 +72,7 @@ static int tps6105x_regulator_probe(struct platform_device *pdev)
 	config.dev = &tps6105x->client->dev;
 	config.init_data = pdata->regulator_data;
 	config.driver_data = tps6105x;
+	config.of_node = pdev->dev.parent->of_node;
 	config.regmap = tps6105x->regmap;
 
 	/* Register regulator with framework */
-- 
2.17.1

Re: [PATCH v2 2/4] regulator: tps6105x: add optional devicetree support

[Mark Brown]

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

On Tue, Nov 19, 2019 at 10:46:09AM -0500, Sven Van Asbroeck wrote:
> Tell the regulator framework to retrieve regulator init
> data from the 'regulator' subnode, or from the parent mfd
> device's platform data.

This and the binding look good.  I think there's no interdependency with
the other patches and I can just apply them?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 2/4] regulator: tps6105x: add optional devicetree support

[Sven Van Asbroeck]

On Tue, Nov 19, 2019 at 1:14 PM Mark Brown <broonie@kernel.org> wrote:
>
> This and the binding look good.  I think there's no interdependency with
> the other patches and I can just apply them?

That's right, things should continue to work as they did before.

Thanks for the guidance on this patch series, appreciate it !

Re: [PATCH v2 2/4] regulator: tps6105x: add optional devicetree support

[Sven Van Asbroeck]

On Tue, Nov 19, 2019 at 1:14 PM Mark Brown <broonie@kernel.org> wrote:
>
> This and the binding look good.  I think there's no interdependency with
> the other patches and I can just apply them?

PS I'm assuming you're accepting the regulator patch, so I'll remove it
from future versions of the patchset, and drop you from To: and Cc:,
to keep the noise down.

Applied "regulator: tps6105x: add optional devicetree support" to the regulator tree

[Mark Brown]

The patch

   regulator: tps6105x: add optional devicetree support

has been applied to the regulator tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-5.5

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From f0a19fa823fb7b1f98d54b22a3ae0e5de88a1e50 Mon Sep 17 00:00:00 2001
From: Sven Van Asbroeck <thesven73@gmail.com>
Date: Tue, 19 Nov 2019 10:46:09 -0500
Subject: [PATCH] regulator: tps6105x: add optional devicetree support

Tell the regulator framework to retrieve regulator init
data from the 'regulator' subnode, or from the parent mfd
device's platform data.

Example:

i2c0 {
	tps61052@33 {
		compatible = "ti,tps61052";
		reg = <0x33>;

		regulator {
			regulator-min-microvolt = <5000000>;
			regulator-max-microvolt = <5000000>;
			regulator-always-on;
		};
	};
};

Tree: next-20191118
Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
Link: https://lore.kernel.org/r/20191119154611.29625-3-TheSven73@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/tps6105x-regulator.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/regulator/tps6105x-regulator.c b/drivers/regulator/tps6105x-regulator.c
index 06059a94f7c6..f8939af0bd2c 100644
--- a/drivers/regulator/tps6105x-regulator.c
+++ b/drivers/regulator/tps6105x-regulator.c
@@ -37,6 +37,7 @@ static struct regulator_ops tps6105x_regulator_ops = {
 
 static const struct regulator_desc tps6105x_regulator_desc = {
 	.name		= "tps6105x-boost",
+	.of_match	= of_match_ptr("regulator"),
 	.ops		= &tps6105x_regulator_ops,
 	.type		= REGULATOR_VOLTAGE,
 	.id		= 0,
@@ -71,6 +72,7 @@ static int tps6105x_regulator_probe(struct platform_device *pdev)
 	config.dev = &tps6105x->client->dev;
 	config.init_data = pdata->regulator_data;
 	config.driver_data = tps6105x;
+	config.of_node = pdev->dev.parent->of_node;
 	config.regmap = tps6105x->regmap;
 
 	/* Register regulator with framework */
-- 
2.20.1

[PATCH v2 3/4] leds: tps6105x: add driver for mfd chip led mode

[Sven Van Asbroeck]

This driver adds support for the led operational mode of the
tps6105x mfd device.

Example usage, devicetree:

i2c0 {
	tps61052@33 {
		compatible = "ti,tps61052";
		reg = <0x33>;

		led {
			label = "tps-led";
		};
	};
};

Example usage, platform data in machine layer:

 #include <linux/mfd/tps6105x.h>

 struct tps6105x_platform_data pdata = {
         .mode = TPS6105X_MODE_TORCH,
         .led_label = "tps-led",
 };

Tree: next-20191118
Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
---
 drivers/leds/Kconfig         | 10 +++++
 drivers/leds/Makefile        |  1 +
 drivers/leds/leds-tps6105x.c | 87 ++++++++++++++++++++++++++++++++++++
 include/linux/mfd/tps6105x.h |  1 +
 4 files changed, 99 insertions(+)
 create mode 100644 drivers/leds/leds-tps6105x.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 4b68520ac251..7c7ceaa824a2 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -836,6 +836,16 @@ config LEDS_LM36274
 	  Say Y to enable the LM36274 LED driver for TI LMU devices.
 	  This supports the LED device LM36274.
 
+config LEDS_TPS6105X
+	tristate "LED support for TI TPS6105X"
+	depends on LEDS_CLASS
+	depends on TPS6105X
+	default y if TPS6105X
+	help
+	  This driver supports TPS61050/TPS61052 led chips.
+	  It is a single boost converter primarily for white LEDs and
+	  audio amplifiers.
+
 comment "LED Triggers"
 source "drivers/leds/trigger/Kconfig"
 
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 2da39e896ce8..d7e1107753fb 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -85,6 +85,7 @@ obj-$(CONFIG_LEDS_LM3601X)		+= leds-lm3601x.o
 obj-$(CONFIG_LEDS_TI_LMU_COMMON)	+= leds-ti-lmu-common.o
 obj-$(CONFIG_LEDS_LM3697)		+= leds-lm3697.o
 obj-$(CONFIG_LEDS_LM36274)		+= leds-lm36274.o
+obj-$(CONFIG_LEDS_TPS6105X)		+= leds-tps6105x.o
 
 # LED SPI Drivers
 obj-$(CONFIG_LEDS_CR0014114)		+= leds-cr0014114.o
diff --git a/drivers/leds/leds-tps6105x.c b/drivers/leds/leds-tps6105x.c
new file mode 100644
index 000000000000..87dbe4846df6
--- /dev/null
+++ b/drivers/leds/leds-tps6105x.c
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/tps6105x.h>
+#include <linux/regmap.h>
+
+struct tps6105x_priv {
+	struct regmap *regmap;
+	struct led_classdev cdev;
+};
+
+static int tps6105x_brightness_set(struct led_classdev *cdev,
+				  enum led_brightness brightness)
+{
+	struct tps6105x_priv *priv = container_of(cdev, struct tps6105x_priv,
+							cdev);
+
+	return regmap_update_bits(priv->regmap, TPS6105X_REG_0,
+		TPS6105X_REG0_TORCHC_MASK,
+		brightness << TPS6105X_REG0_TORCHC_SHIFT);
+}
+
+static const char *label_from_dt(struct device *dev)
+{
+	struct device_node *led =
+		of_get_child_by_name(dev->parent->of_node, "led");
+	const char *label;
+
+	if (!led) {
+		dev_err(dev, "led node not found");
+		return NULL;
+	}
+	if (of_property_read_string(led, "label", &label))
+		label = NULL;
+	of_node_put(led);
+
+	return label;
+}
+
+static int tps6105x_led_probe(struct platform_device *pdev)
+{
+	struct tps6105x *tps6105x = dev_get_platdata(&pdev->dev);
+	struct tps6105x_platform_data *pdata = tps6105x->pdata;
+	struct tps6105x_priv *priv;
+	const char *label;
+	int ret;
+
+	/* This instance is not set for torch mode so bail out */
+	if (pdata->mode != TPS6105X_MODE_TORCH) {
+		dev_info(&pdev->dev,
+			"chip not in torch mode, exit probe");
+		return -EINVAL;
+	}
+
+	label = pdata->led_label ?: label_from_dt(&pdev->dev);
+	label = label ?: "tps6105x";
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	priv->regmap = tps6105x->regmap;
+	priv->cdev.name = label;
+	priv->cdev.brightness_set_blocking = tps6105x_brightness_set;
+	priv->cdev.max_brightness = 7;
+
+	ret = regmap_update_bits(tps6105x->regmap, TPS6105X_REG_0,
+		TPS6105X_REG0_MODE_MASK | TPS6105X_REG0_TORCHC_MASK,
+		TPS6105X_REG0_MODE_TORCH << TPS6105X_REG0_MODE_SHIFT);
+	if (ret)
+		return ret;
+
+	return devm_led_classdev_register(&pdev->dev, &priv->cdev);
+}
+
+static struct platform_driver led_driver = {
+	.probe = tps6105x_led_probe,
+	.driver = {
+		.name = "tps6105x-leds",
+	},
+};
+
+module_platform_driver(led_driver);
+
+MODULE_DESCRIPTION("TPS6105x led driver");
+MODULE_AUTHOR("Sven Van Asbroeck <TheSven73@gmail.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/tps6105x.h b/include/linux/mfd/tps6105x.h
index b1313411ef09..287a67e004f8 100644
--- a/include/linux/mfd/tps6105x.h
+++ b/include/linux/mfd/tps6105x.h
@@ -78,6 +78,7 @@ enum tps6105x_mode {
 struct tps6105x_platform_data {
 	enum tps6105x_mode mode;
 	struct regulator_init_data *regulator_data;
+	const char *led_label;
 };
 
 /**
-- 
2.17.1

Re: [PATCH v2 3/4] leds: tps6105x: add driver for mfd chip led mode

[Dan Murphy]

Sven

On 11/19/19 9:46 AM, Sven Van Asbroeck wrote:
> This driver adds support for the led operational mode of the
> tps6105x mfd device.
How many LEDs does this device support?
>
> Example usage, devicetree:
>
> i2c0 {
> 	tps61052@33 {
> 		compatible = "ti,tps61052";
> 		reg = <0x33>;
>
> 		led {
> 			label = "tps-led";
> 		};
> 	};
> };
>
> Example usage, platform data in machine layer:
>
>   #include <linux/mfd/tps6105x.h>
>
>   struct tps6105x_platform_data pdata = {
>           .mode = TPS6105X_MODE_TORCH,
>           .led_label = "tps-led",
>   };
>
> Tree: next-20191118
> Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
> ---
>   drivers/leds/Kconfig         | 10 +++++
>   drivers/leds/Makefile        |  1 +
>   drivers/leds/leds-tps6105x.c | 87 ++++++++++++++++++++++++++++++++++++
>   include/linux/mfd/tps6105x.h |  1 +
>   4 files changed, 99 insertions(+)
>   create mode 100644 drivers/leds/leds-tps6105x.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 4b68520ac251..7c7ceaa824a2 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -836,6 +836,16 @@ config LEDS_LM36274
>   	  Say Y to enable the LM36274 LED driver for TI LMU devices.
>   	  This supports the LED device LM36274.
>   
> +config LEDS_TPS6105X
> +	tristate "LED support for TI TPS6105X"
> +	depends on LEDS_CLASS
> +	depends on TPS6105X
> +	default y if TPS6105X
> +	help
> +	  This driver supports TPS61050/TPS61052 led chips.
> +	  It is a single boost converter primarily for white LEDs and
> +	  audio amplifiers.
> +
>   comment "LED Triggers"
>   source "drivers/leds/trigger/Kconfig"
>   
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 2da39e896ce8..d7e1107753fb 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -85,6 +85,7 @@ obj-$(CONFIG_LEDS_LM3601X)		+= leds-lm3601x.o
>   obj-$(CONFIG_LEDS_TI_LMU_COMMON)	+= leds-ti-lmu-common.o
>   obj-$(CONFIG_LEDS_LM3697)		+= leds-lm3697.o
>   obj-$(CONFIG_LEDS_LM36274)		+= leds-lm36274.o
> +obj-$(CONFIG_LEDS_TPS6105X)		+= leds-tps6105x.o
>   
>   # LED SPI Drivers
>   obj-$(CONFIG_LEDS_CR0014114)		+= leds-cr0014114.o
> diff --git a/drivers/leds/leds-tps6105x.c b/drivers/leds/leds-tps6105x.c
> new file mode 100644
> index 000000000000..87dbe4846df6
> --- /dev/null
> +++ b/drivers/leds/leds-tps6105x.c
> @@ -0,0 +1,87 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/leds.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/mfd/tps6105x.h>
> +#include <linux/regmap.h>
> +
> +struct tps6105x_priv {
> +	struct regmap *regmap;
> +	struct led_classdev cdev;
> +};
> +
> +static int tps6105x_brightness_set(struct led_classdev *cdev,
> +				  enum led_brightness brightness)
> +{
> +	struct tps6105x_priv *priv = container_of(cdev, struct tps6105x_priv,
> +							cdev);
> +
> +	return regmap_update_bits(priv->regmap, TPS6105X_REG_0,
> +		TPS6105X_REG0_TORCHC_MASK,
> +		brightness << TPS6105X_REG0_TORCHC_SHIFT);
> +}
> +
> +static const char *label_from_dt(struct device *dev)
> +{
> +	struct device_node *led =
> +		of_get_child_by_name(dev->parent->of_node, "led");
Prefer device_* calls as opposed to of_* calls.
> +	const char *label;
> +
> +	if (!led) {
> +		dev_err(dev, "led node not found");
> +		return NULL;
> +	}
> +	if (of_property_read_string(led, "label", &label))
same as above
> +		label = NULL;
> +	of_node_put(led);
> +
> +	return label;
> +}
> +
> +static int tps6105x_led_probe(struct platform_device *pdev)
> +{
> +	struct tps6105x *tps6105x = dev_get_platdata(&pdev->dev);
> +	struct tps6105x_platform_data *pdata = tps6105x->pdata;
> +	struct tps6105x_priv *priv;
> +	const char *label;
> +	int ret;
> +
> +	/* This instance is not set for torch mode so bail out */
> +	if (pdata->mode != TPS6105X_MODE_TORCH) {
> +		dev_info(&pdev->dev,
> +			"chip not in torch mode, exit probe");
> +		return -EINVAL;
> +	}
> +
> +	label = pdata->led_label ?: label_from_dt(&pdev->dev);

Since this is a new driver do we really have to continue to use the 
pdata for the init

data?  Can't we just get the label from the DT node now like other drivers?

Dan

Re: [PATCH v2 3/4] leds: tps6105x: add driver for mfd chip led mode

[Sven Van Asbroeck]

On Tue, Nov 19, 2019 at 1:33 PM Dan Murphy <dmurphy@ti.com> wrote:
>
> How many LEDs does this device support?

Just a single LED.

> > +     struct device_node *led =
> > +             of_get_child_by_name(dev->parent->of_node, "led");
> Prefer device_* calls as opposed to of_* calls.

So do I. But because of Mark Brown's suggestions, the mfd children
now have to grab a named sub-node from their parent.
In this case, we grab the parent subnode named 'led' and fetch
the label from it.
(https://lkml.org/lkml/2019/11/18/802)
Perhaps there is a way to accomplish this this with device_*
calls?

> > +     label = pdata->led_label ?: label_from_dt(&pdev->dev);
>
> Since this is a new driver do we really have to continue to use the
> pdata for the init
>
> data?  Can't we just get the label from the DT node now like other drivers?

Yes we can, but pdata users would not be able to specify the label name.
Until this patch set, pdata was the only way to use the driver.
Of course, no-one is requesting or requiring this. So I guess
it can be dropped.

Re: [PATCH v2 3/4] leds: tps6105x: add driver for mfd chip led mode

[Sven Van Asbroeck]

Dan / Jacek,

On Tue, Nov 19, 2019 at 1:33 PM Dan Murphy <dmurphy@ti.com> wrote:
>
> Prefer device_* calls as opposed to of_* calls.

Ok, I see now how this is done on lp8860 and lm3692x.

So for this led driver to be correct, it should:
- use device_* calls
- use led_init_data (i.e. devm_led_classdev_register_ext)
- drop pdata support for the led subdriver (nice to have)

Unfortunately I can't, because the required infrastructure isn't present in
the 4.19 vendor kernel I'm using.

I only have a single tps6105x chip in my target system, so I don't have
a requirement for devicetree led naming.

So for v3 I will hard-code the led name to 'tps6105x::torch' which should
comply with the naming convention.

[PATCH v2 4/4] dt-bindings: mfd: update TI tps6105x chip bindings

[Sven Van Asbroeck]

The driver has been extended to optionally get its operational
mode, regulator init data and led label from the devicetree.

Tree: next-20191118
Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
---
 .../devicetree/bindings/mfd/tps6105x.txt      | 42 ++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mfd/tps6105x.txt b/Documentation/devicetree/bindings/mfd/tps6105x.txt
index 93602c7a19c8..ab5d4c52074f 100644
--- a/Documentation/devicetree/bindings/mfd/tps6105x.txt
+++ b/Documentation/devicetree/bindings/mfd/tps6105x.txt
@@ -7,11 +7,51 @@ Required properties:
 - compatible:		"ti,tps61050" or "ti,tps61052"
 - reg:			Specifies the I2C slave address
 
-Example:
+Optional sub-node:
+
+This subnode selects the chip's operational mode.
+There can be at most one single available subnode.
+
+- regulator: presence of this sub-node puts the chip in regulator mode.
+	see Documentation/devicetree/bindings/regulator/regulator.txt
+
+- led: presence of this sub-node puts the chip in led mode.
+	Optional properties:
+		- label: see Documentation/devicetree/bindings/leds/common.txt
+
+Example (GPIO operation only):
+
+i2c0 {
+	tps61052@33 {
+		compatible = "ti,tps61052";
+		reg = <0x33>;
+	};
+};
+
+Example (GPIO + regulator operation):
 
 i2c0 {
 	tps61052@33 {
 		compatible = "ti,tps61052";
 		reg = <0x33>;
+
+		regulator {
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			regulator-always-on;
+		};
+	};
+};
+
+Example (GPIO + led operation):
+
+i2c0 {
+	tps61052@33 {
+		compatible = "ti,tps61052";
+		reg = <0x33>;
+
+		led {
+			label = "tps-torch";
+		};
 	};
 };
-- 
2.17.1

Re: [PATCH v2 4/4] dt-bindings: mfd: update TI tps6105x chip bindings

[Dan Murphy]

Sven

On 11/19/19 9:46 AM, Sven Van Asbroeck wrote:
> The driver has been extended to optionally get its operational
> mode, regulator init data and led label from the devicetree.
>
> Tree: next-20191118
> Signed-off-by: Sven Van Asbroeck <TheSven73@gmail.com>
> ---
>   .../devicetree/bindings/mfd/tps6105x.txt      | 42 ++++++++++++++++++-
>   1 file changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/tps6105x.txt b/Documentation/devicetree/bindings/mfd/tps6105x.txt
> index 93602c7a19c8..ab5d4c52074f 100644
> --- a/Documentation/devicetree/bindings/mfd/tps6105x.txt
> +++ b/Documentation/devicetree/bindings/mfd/tps6105x.txt
> @@ -7,11 +7,51 @@ Required properties:
>   - compatible:		"ti,tps61050" or "ti,tps61052"
>   - reg:			Specifies the I2C slave address
>   
> -Example:
> +Optional sub-node:
> +
> +This subnode selects the chip's operational mode.
> +There can be at most one single available subnode.
> +
> +- regulator: presence of this sub-node puts the chip in regulator mode.
> +	see Documentation/devicetree/bindings/regulator/regulator.txt
> +
> +- led: presence of this sub-node puts the chip in led mode.
> +	Optional properties:
> +		- label: see Documentation/devicetree/bindings/leds/common.txt
> +
> +Example (GPIO operation only):
> +
> +i2c0 {
> +	tps61052@33 {
> +		compatible = "ti,tps61052";
> +		reg = <0x33>;
> +	};
> +};
> +
> +Example (GPIO + regulator operation):
>   
>   i2c0 {
>   	tps61052@33 {
>   		compatible = "ti,tps61052";
>   		reg = <0x33>;
> +
> +		regulator {
> +			regulator-min-microvolt = <5000000>;
> +			regulator-max-microvolt = <5000000>;
> +			regulator-always-on;
> +		};
> +	};
> +};
> +
> +Example (GPIO + led operation):
What part of the example is GPIO? Is that the default function?
> +
> +i2c0 {
> +	tps61052@33 {
> +		compatible = "ti,tps61052";
> +		reg = <0x33>;
> +
> +		led {
> +			label = "tps-torch";

function and color examples?


> +		};
>   	};
>   };

Re: [PATCH v2 4/4] dt-bindings: mfd: update TI tps6105x chip bindings

[Sven Van Asbroeck]

On Tue, Nov 19, 2019 at 1:37 PM Dan Murphy <dmurphy@ti.com> wrote:
>
> > +
> > +Example (GPIO + led operation):
> What part of the example is GPIO? Is that the default function?

The gpio function is always available by default.
The mfd driver always adds the gpio mfd_cell.
But no-one has ever implemented a mfd sub-driver for gpio.

> > +
> > +i2c0 {
> > +     tps61052@33 {
> > +             compatible = "ti,tps61052";
> > +             reg = <0x33>;
> > +
> > +             led {
> > +                     label = "tps-torch";
>
> function and color examples?

No function, no colour. This is a simple led control with 8 intensity
steps. We use it as a led torch on an industrial device.

Re: [PATCH v2 4/4] dt-bindings: mfd: update TI tps6105x chip bindings

[Jacek Anaszewski]

Hi Sven,

On 11/19/19 8:03 PM, Sven Van Asbroeck wrote:
> On Tue, Nov 19, 2019 at 1:37 PM Dan Murphy <dmurphy@ti.com> wrote:
>>
>>> +
>>> +Example (GPIO + led operation):
>> What part of the example is GPIO? Is that the default function?
> 
> The gpio function is always available by default.
> The mfd driver always adds the gpio mfd_cell.
> But no-one has ever implemented a mfd sub-driver for gpio.
> 
>>> +
>>> +i2c0 {
>>> +     tps61052@33 {
>>> +             compatible = "ti,tps61052";
>>> +             reg = <0x33>;
>>> +
>>> +             led {
>>> +                     label = "tps-torch";
>>
>> function and color examples?
> 
> No function, no colour. This is a simple led control with 8 intensity
> steps. We use it as a led torch on an industrial device.

label DT property was recently deprecated. We now encourage using
'function' and/or 'color'. Please refer to
Documentation/devicetree/bindings/leds/common.txt.

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH v2 4/4] dt-bindings: mfd: update TI tps6105x chip bindings

[Sven Van Asbroeck]

On Tue, Nov 19, 2019 at 2:38 PM Jacek Anaszewski
<jacek.anaszewski@gmail.com> wrote:
>
> label DT property was recently deprecated. We now encourage using
> 'function' and/or 'color'. Please refer to
> Documentation/devicetree/bindings/leds/common.txt.

Ah, I see.

I'm actually back-porting this from a 4.19 vendor kernel, which I believe
does not have function/color support. If I introduce it here, I wouldn't
be able to test it easily, and I'd have to forward/back-port.

All I need is a single led in the system. So I'll just remove the 'label'
code/description, and hard-code the led name to 'tps6105x'. If
anyone wants to use this driver and wants named leds, they can
always add it themselves.

Jacek and Dan, does that satisfy your concerns?