All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/8] leds: Add AXP20X CHGLED
@ 2019-02-15 11:50 ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

This series add support for CHGLED pin found on most of
X-Powers PMICs.

Changes for v2:
- use mutex when accessing hardware registers
- fixed LED naming style
- fixed some typos
- fix variable types when showing attributes
- remove useless private data
- fix binding documentation
- add device consumers

Stefan Mavrodiev (8):
  leds: Add support for AXP20X CHGLED
  mfd: axp20x: Add axp20x-led cell
  dt-bindings: leds: Add binding for axp20x-led device driver
  arm64: dts: allwinner: axp803: add charge led node
  arm64: dts: allwinner: Enable AXP803 CHGLED for Olimex boards
  arm: dts: axpxx: add charge led node
  ARM: dts: sun7i: Enable AXP209 CHGLED for Olimex boards
  ARM: dts: sun8i: a33: Enable AXP223 CHGLED for A33-OLinuXino

 .../devicetree/bindings/leds/leds-axp20x.txt  |  74 +++++
 arch/arm/boot/dts/axp209.dtsi                 |   5 +
 arch/arm/boot/dts/axp22x.dtsi                 |   5 +
 arch/arm/boot/dts/axp81x.dtsi                 |   5 +
 .../arm/boot/dts/sun7i-a20-olimex-som-evb.dts |   6 +
 .../boot/dts/sun7i-a20-olimex-som204-evb.dts  |   6 +
 .../boot/dts/sun7i-a20-olinuxino-lime2.dts    |   6 +
 .../boot/dts/sun7i-a20-olinuxino-micro.dts    |   6 +
 arch/arm/boot/dts/sun8i-a33-olinuxino.dts     |   6 +
 arch/arm64/boot/dts/allwinner/axp803.dtsi     |   5 +
 .../dts/allwinner/sun50i-a64-olinuxino.dts    |   6 +
 .../boot/dts/allwinner/sun50i-a64-teres-i.dts |   6 +
 drivers/leds/Kconfig                          |  10 +
 drivers/leds/Makefile                         |   1 +
 drivers/leds/leds-axp20x.c                    | 291 ++++++++++++++++++
 drivers/mfd/axp20x.c                          |  24 +-
 16 files changed, 461 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-axp20x.txt
 create mode 100644 drivers/leds/leds-axp20x.c

-- 
2.17.1

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

* [PATCH v2 0/8] leds: Add AXP20X CHGLED
@ 2019-02-15 11:50 ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

This series add support for CHGLED pin found on most of
X-Powers PMICs.

Changes for v2:
- use mutex when accessing hardware registers
- fixed LED naming style
- fixed some typos
- fix variable types when showing attributes
- remove useless private data
- fix binding documentation
- add device consumers

Stefan Mavrodiev (8):
  leds: Add support for AXP20X CHGLED
  mfd: axp20x: Add axp20x-led cell
  dt-bindings: leds: Add binding for axp20x-led device driver
  arm64: dts: allwinner: axp803: add charge led node
  arm64: dts: allwinner: Enable AXP803 CHGLED for Olimex boards
  arm: dts: axpxx: add charge led node
  ARM: dts: sun7i: Enable AXP209 CHGLED for Olimex boards
  ARM: dts: sun8i: a33: Enable AXP223 CHGLED for A33-OLinuXino

 .../devicetree/bindings/leds/leds-axp20x.txt  |  74 +++++
 arch/arm/boot/dts/axp209.dtsi                 |   5 +
 arch/arm/boot/dts/axp22x.dtsi                 |   5 +
 arch/arm/boot/dts/axp81x.dtsi                 |   5 +
 .../arm/boot/dts/sun7i-a20-olimex-som-evb.dts |   6 +
 .../boot/dts/sun7i-a20-olimex-som204-evb.dts  |   6 +
 .../boot/dts/sun7i-a20-olinuxino-lime2.dts    |   6 +
 .../boot/dts/sun7i-a20-olinuxino-micro.dts    |   6 +
 arch/arm/boot/dts/sun8i-a33-olinuxino.dts     |   6 +
 arch/arm64/boot/dts/allwinner/axp803.dtsi     |   5 +
 .../dts/allwinner/sun50i-a64-olinuxino.dts    |   6 +
 .../boot/dts/allwinner/sun50i-a64-teres-i.dts |   6 +
 drivers/leds/Kconfig                          |  10 +
 drivers/leds/Makefile                         |   1 +
 drivers/leds/leds-axp20x.c                    | 291 ++++++++++++++++++
 drivers/mfd/axp20x.c                          |  24 +-
 16 files changed, 461 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-axp20x.txt
 create mode 100644 drivers/leds/leds-axp20x.c

-- 
2.17.1


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

* [PATCH v2 0/8] leds: Add AXP20X CHGLED
@ 2019-02-15 11:50 ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

This series add support for CHGLED pin found on most of
X-Powers PMICs.

Changes for v2:
- use mutex when accessing hardware registers
- fixed LED naming style
- fixed some typos
- fix variable types when showing attributes
- remove useless private data
- fix binding documentation
- add device consumers

Stefan Mavrodiev (8):
  leds: Add support for AXP20X CHGLED
  mfd: axp20x: Add axp20x-led cell
  dt-bindings: leds: Add binding for axp20x-led device driver
  arm64: dts: allwinner: axp803: add charge led node
  arm64: dts: allwinner: Enable AXP803 CHGLED for Olimex boards
  arm: dts: axpxx: add charge led node
  ARM: dts: sun7i: Enable AXP209 CHGLED for Olimex boards
  ARM: dts: sun8i: a33: Enable AXP223 CHGLED for A33-OLinuXino

 .../devicetree/bindings/leds/leds-axp20x.txt  |  74 +++++
 arch/arm/boot/dts/axp209.dtsi                 |   5 +
 arch/arm/boot/dts/axp22x.dtsi                 |   5 +
 arch/arm/boot/dts/axp81x.dtsi                 |   5 +
 .../arm/boot/dts/sun7i-a20-olimex-som-evb.dts |   6 +
 .../boot/dts/sun7i-a20-olimex-som204-evb.dts  |   6 +
 .../boot/dts/sun7i-a20-olinuxino-lime2.dts    |   6 +
 .../boot/dts/sun7i-a20-olinuxino-micro.dts    |   6 +
 arch/arm/boot/dts/sun8i-a33-olinuxino.dts     |   6 +
 arch/arm64/boot/dts/allwinner/axp803.dtsi     |   5 +
 .../dts/allwinner/sun50i-a64-olinuxino.dts    |   6 +
 .../boot/dts/allwinner/sun50i-a64-teres-i.dts |   6 +
 drivers/leds/Kconfig                          |  10 +
 drivers/leds/Makefile                         |   1 +
 drivers/leds/leds-axp20x.c                    | 291 ++++++++++++++++++
 drivers/mfd/axp20x.c                          |  24 +-
 16 files changed, 461 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-axp20x.txt
 create mode 100644 drivers/leds/leds-axp20x.c

-- 
2.17.1


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

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

* [PATCH v2 1/8] leds: Add support for AXP20X CHGLED
  2019-02-15 11:50 ` Stefan Mavrodiev
  (?)
@ 2019-02-15 11:50   ` Stefan Mavrodiev
  -1 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

Most of AXP20x PMIC chips have built-in battery charger with LED indicator.
The LED can be controlled ether by the charger or manually by a register.

The default is (except for AXP209) manual control, which makes this LED
useless, since there is no device driver.

The driver rely on AXP20X MFD driver.

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 drivers/leds/Kconfig       |  10 ++
 drivers/leds/Makefile      |   1 +
 drivers/leds/leds-axp20x.c | 291 +++++++++++++++++++++++++++++++++++++
 3 files changed, 302 insertions(+)
 create mode 100644 drivers/leds/leds-axp20x.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index a72f97fca57b..82dce9063d41 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -766,6 +766,16 @@ config LEDS_NIC78BX
 	  To compile this driver as a module, choose M here: the module
 	  will be called leds-nic78bx.
 
+config LEDS_AXP20X
+	tristate "LED support for X-Powers PMICs"
+	depends on MFD_AXP20X
+	help
+	  This option enables support for CHGLED found on most of X-Powers
+	  PMICs.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called leds-axp20x.
+
 comment "LED Triggers"
 source "drivers/leds/trigger/Kconfig"
 
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 4c1b0054f379..d3fb76e119d8 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -79,6 +79,7 @@ obj-$(CONFIG_LEDS_MT6323)		+= leds-mt6323.o
 obj-$(CONFIG_LEDS_LM3692X)		+= leds-lm3692x.o
 obj-$(CONFIG_LEDS_SC27XX_BLTC)		+= leds-sc27xx-bltc.o
 obj-$(CONFIG_LEDS_LM3601X)		+= leds-lm3601x.o
+obj-$(CONFIG_LEDS_AXP20X)		+= leds-axp20x.o
 
 # LED SPI Drivers
 obj-$(CONFIG_LEDS_CR0014114)		+= leds-cr0014114.o
diff --git a/drivers/leds/leds-axp20x.c b/drivers/leds/leds-axp20x.c
new file mode 100644
index 000000000000..2d5ae1c085f0
--- /dev/null
+++ b/drivers/leds/leds-axp20x.c
@@ -0,0 +1,291 @@
+// SPDX-License-Identifier: GPL-2.0+
+//
+// Copyright 2019 Stefan Mavrodiev <stefan@olimex.com>
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/leds.h>
+#include <linux/mfd/axp20x.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <uapi/linux/uleds.h>
+
+
+#define AXP20X_CHGLED_CTRL_REG		AXP20X_OFF_CTRL
+#define AXP20X_CHGLED_FUNC_MASK			GENMASK(5, 4)
+#define AXP20X_CHGLED_FUNC_OFF			(0 << 4)
+#define AXP20X_CHGLED_FUNC_1HZ			(1 << 4)
+#define AXP20X_CHGLED_FUNC_4HZ			(2 << 4)
+#define AXP20X_CHGLED_FUNC_FULL			(3 << 4)
+#define AXP20X_CHGLED_CTRL_MASK			BIT(3)
+#define AXP20X_CHGLED_CTRL_MANUAL		0
+#define AXP20X_CHGLED_CTRL_CHARGER		1
+#define AXP20X_CHGLED_CTRL(_ctrl)		(_ctrl << 3)
+
+#define AXP20X_CHGLED_MODE_REG		AXP20X_CHRG_CTRL2
+#define AXP20X_CHGLED_MODE_MASK			BIT(4)
+#define AXP20X_CHGLED_MODE_A			0
+#define AXP20X_CHGLED_MODE_B			1
+#define AXP20X_CHGLED_MODE(_mode)		(_mode << 4)
+
+struct axp20x_led {
+	char			name[LED_MAX_NAME_SIZE];
+	struct led_classdev	cdev;
+	struct mutex		lock;
+	u8			mode : 1;
+	u8			ctrl : 1;
+	u8			ctrl_inverted : 1;
+	struct axp20x_dev	*axp20x;
+};
+
+static inline struct axp20x_led *to_axp20x_led(struct led_classdev *cdev)
+{
+	return container_of(cdev, struct axp20x_led, cdev);
+}
+
+static int axp20x_led_setup(struct axp20x_led *priv)
+{
+	int ret;
+	u8 val;
+
+	/* Invert the logic, if necessary */
+	val = priv->ctrl ^ priv->ctrl_inverted;
+
+	mutex_lock(&priv->lock);
+	ret = regmap_update_bits(priv->axp20x->regmap, AXP20X_CHGLED_CTRL_REG,
+				 AXP20X_CHGLED_CTRL_MASK,
+				 AXP20X_CHGLED_CTRL(val));
+	if (ret < 0)
+		goto out;
+
+	ret = regmap_update_bits(priv->axp20x->regmap, AXP20X_CHGLED_MODE_REG,
+				 AXP20X_CHGLED_MODE_MASK,
+				 AXP20X_CHGLED_MODE(priv->mode));
+out:
+	mutex_unlock(&priv->lock);
+	return ret;
+}
+
+static ssize_t control_show(struct device *dev, struct device_attribute *attr,
+			    char *buf)
+{
+	struct led_classdev *cdev = dev_get_drvdata(dev);
+	struct axp20x_led *priv = to_axp20x_led(cdev);
+
+	return sprintf(buf, "%u\n", priv->ctrl);
+}
+
+static ssize_t control_store(struct device *dev, struct device_attribute *attr,
+			     const char *buf, size_t size)
+{
+	struct led_classdev *cdev = dev_get_drvdata(dev);
+	struct axp20x_led *priv = to_axp20x_led(cdev);
+	unsigned long val;
+	int ret;
+
+	ret = kstrtoul(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	/**
+	 * Supported values are:
+	 *   - 0 : Manual control
+	 *   - 1 : Charger control
+	 */
+	if (val > 1)
+		return -EINVAL;
+
+	priv->ctrl = val;
+
+	return axp20x_led_setup(priv) ? : size;
+}
+static DEVICE_ATTR_RW(control);
+
+static ssize_t mode_show(struct device *dev, struct device_attribute *attr,
+			 char *buf)
+{
+	struct led_classdev *cdev = dev_get_drvdata(dev);
+	struct axp20x_led *priv = to_axp20x_led(cdev);
+
+	return sprintf(buf, "%u\n", priv->mode);
+}
+
+static ssize_t mode_store(struct device *dev, struct device_attribute *attr,
+			  const char *buf, size_t size)
+{
+	struct led_classdev *cdev = dev_get_drvdata(dev);
+	struct axp20x_led *priv = to_axp20x_led(cdev);
+	unsigned long val;
+	int ret;
+
+	ret = kstrtoul(buf, 0, &val);
+	if (ret)
+		return ret;
+	/**
+	 * Supported values are:
+	 *   - 0 : Mode A
+	 *   - 1 : Mode B
+	 */
+	if (val > 1)
+		return -EINVAL;
+
+	priv->mode = val;
+
+	return axp20x_led_setup(priv) ? : size;
+}
+static DEVICE_ATTR_RW(mode);
+
+static struct attribute *axp20x_led_attrs[] = {
+	&dev_attr_control.attr,
+	&dev_attr_mode.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(axp20x_led);
+
+enum led_brightness axp20x_led_brightness_get(struct led_classdev *cdev)
+{
+	struct axp20x_led *priv = to_axp20x_led(cdev);
+	u32 val;
+	int ret;
+
+	mutex_lock(&priv->lock);
+	ret = regmap_read(priv->axp20x->regmap, AXP20X_CHGLED_CTRL_REG, &val);
+	mutex_unlock(&priv->lock);
+	if (ret < 0)
+		return LED_OFF;
+
+	return (val & AXP20X_CHGLED_FUNC_FULL) ? LED_FULL : LED_OFF;
+}
+
+static int axp20x_led_brightness_set_blocking(struct led_classdev *cdev,
+					      enum led_brightness brightness)
+{
+	struct axp20x_led *priv = to_axp20x_led(cdev);
+	int ret = 0;
+
+	mutex_lock(&priv->lock);
+	ret = regmap_update_bits(priv->axp20x->regmap,
+				 AXP20X_CHGLED_CTRL_REG,
+				 AXP20X_CHGLED_FUNC_MASK,
+				 (brightness) ?
+				 AXP20X_CHGLED_FUNC_FULL :
+				 AXP20X_CHGLED_FUNC_OFF);
+	mutex_unlock(&priv->lock);
+
+	return ret;
+}
+
+static int axp20x_led_parse_dt(struct axp20x_led *priv, struct device_node *np)
+{
+	const char *str;
+	u8 value;
+	int ret = 0;
+
+	str = of_get_property(np, "label", NULL);
+	if (!str)
+		snprintf(priv->name, sizeof(priv->name), "axp20x::");
+	else
+		snprintf(priv->name, sizeof(priv->name), "axp20x:%s", str);
+	priv->cdev.name = priv->name;
+
+	priv->cdev.default_trigger = of_get_property(np,
+						     "linux,default-trigger",
+						     NULL);
+
+	if (!of_property_read_u8(np, "x-powers,charger-mode", &value)) {
+		priv->ctrl = AXP20X_CHGLED_CTRL_CHARGER;
+		priv->mode = (value < 2) ? value : 0;
+	} else {
+		priv->ctrl = AXP20X_CHGLED_CTRL_MANUAL;
+	}
+
+	str = of_get_property(np, "default-state", NULL);
+	if (str) {
+		if (!strcmp(str, "keep")) {
+			ret = axp20x_led_brightness_get(&priv->cdev);
+			if (ret < 0)
+				return ret;
+			priv->cdev.brightness = ret;
+		} else if (!strcmp(str, "on")) {
+			ret = axp20x_led_brightness_set_blocking(&priv->cdev,
+								 LED_FULL);
+		} else  {
+			ret = axp20x_led_brightness_set_blocking(&priv->cdev,
+								 LED_OFF);
+		}
+	}
+
+	return ret;
+}
+
+static const struct of_device_id axp20x_led_of_match[] = {
+	{ .compatible = "x-powers,axp20x-led" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, axp20x_led_of_match);
+
+static int axp20x_led_probe(struct platform_device *pdev)
+{
+	struct axp20x_led *priv;
+	int ret;
+
+	if (!of_device_is_available(pdev->dev.of_node))
+		return -ENODEV;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(struct axp20x_led),
+			    GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->axp20x = dev_get_drvdata(pdev->dev.parent);
+	if (!priv->axp20x) {
+		dev_err(&pdev->dev, "Failed to get parent data\n");
+		return -ENXIO;
+	}
+
+	mutex_init(&priv->lock);
+
+	priv->cdev.brightness_set_blocking = axp20x_led_brightness_set_blocking;
+	priv->cdev.brightness_get = axp20x_led_brightness_get;
+	priv->cdev.groups = axp20x_led_groups;
+
+	ret = axp20x_led_parse_dt(priv, pdev->dev.of_node);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to set parameters\n");
+		return ret;
+	}
+
+	/**
+	 * For some reason in AXP209 the bit that controls CHGLED is with
+	 * inverted logic compared to all other PMICs.
+	 * If the PMIC is actually AXP209, set inverted flag and later use it
+	 * when configuring the LED.
+	 */
+	if (priv->axp20x->variant == AXP209_ID)
+		priv->ctrl_inverted = 1;
+
+	ret =  axp20x_led_setup(priv);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to configure led");
+		return ret;
+	}
+
+	return devm_led_classdev_register(&pdev->dev, &priv->cdev);
+}
+
+static struct platform_driver axp20x_led_driver = {
+	.driver = {
+		.name	= "axp20x-led",
+		.of_match_table = of_match_ptr(axp20x_led_of_match),
+	},
+	.probe = axp20x_led_probe,
+};
+
+module_platform_driver(axp20x_led_driver);
+
+MODULE_AUTHOR("Stefan Mavrodiev <stefan@olimex.com");
+MODULE_DESCRIPTION("X-Powers PMIC CHGLED driver");
+MODULE_LICENSE("GPL");
-- 
2.17.1

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

* [PATCH v2 1/8] leds: Add support for AXP20X CHGLED
@ 2019-02-15 11:50   ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

Most of AXP20x PMIC chips have built-in battery charger with LED indicator.
The LED can be controlled ether by the charger or manually by a register.

The default is (except for AXP209) manual control, which makes this LED
useless, since there is no device driver.

The driver rely on AXP20X MFD driver.

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 drivers/leds/Kconfig       |  10 ++
 drivers/leds/Makefile      |   1 +
 drivers/leds/leds-axp20x.c | 291 +++++++++++++++++++++++++++++++++++++
 3 files changed, 302 insertions(+)
 create mode 100644 drivers/leds/leds-axp20x.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index a72f97fca57b..82dce9063d41 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -766,6 +766,16 @@ config LEDS_NIC78BX
 	  To compile this driver as a module, choose M here: the module
 	  will be called leds-nic78bx.
 
+config LEDS_AXP20X
+	tristate "LED support for X-Powers PMICs"
+	depends on MFD_AXP20X
+	help
+	  This option enables support for CHGLED found on most of X-Powers
+	  PMICs.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called leds-axp20x.
+
 comment "LED Triggers"
 source "drivers/leds/trigger/Kconfig"
 
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 4c1b0054f379..d3fb76e119d8 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -79,6 +79,7 @@ obj-$(CONFIG_LEDS_MT6323)		+= leds-mt6323.o
 obj-$(CONFIG_LEDS_LM3692X)		+= leds-lm3692x.o
 obj-$(CONFIG_LEDS_SC27XX_BLTC)		+= leds-sc27xx-bltc.o
 obj-$(CONFIG_LEDS_LM3601X)		+= leds-lm3601x.o
+obj-$(CONFIG_LEDS_AXP20X)		+= leds-axp20x.o
 
 # LED SPI Drivers
 obj-$(CONFIG_LEDS_CR0014114)		+= leds-cr0014114.o
diff --git a/drivers/leds/leds-axp20x.c b/drivers/leds/leds-axp20x.c
new file mode 100644
index 000000000000..2d5ae1c085f0
--- /dev/null
+++ b/drivers/leds/leds-axp20x.c
@@ -0,0 +1,291 @@
+// SPDX-License-Identifier: GPL-2.0+
+//
+// Copyright 2019 Stefan Mavrodiev <stefan@olimex.com>
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/leds.h>
+#include <linux/mfd/axp20x.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <uapi/linux/uleds.h>
+
+
+#define AXP20X_CHGLED_CTRL_REG		AXP20X_OFF_CTRL
+#define AXP20X_CHGLED_FUNC_MASK			GENMASK(5, 4)
+#define AXP20X_CHGLED_FUNC_OFF			(0 << 4)
+#define AXP20X_CHGLED_FUNC_1HZ			(1 << 4)
+#define AXP20X_CHGLED_FUNC_4HZ			(2 << 4)
+#define AXP20X_CHGLED_FUNC_FULL			(3 << 4)
+#define AXP20X_CHGLED_CTRL_MASK			BIT(3)
+#define AXP20X_CHGLED_CTRL_MANUAL		0
+#define AXP20X_CHGLED_CTRL_CHARGER		1
+#define AXP20X_CHGLED_CTRL(_ctrl)		(_ctrl << 3)
+
+#define AXP20X_CHGLED_MODE_REG		AXP20X_CHRG_CTRL2
+#define AXP20X_CHGLED_MODE_MASK			BIT(4)
+#define AXP20X_CHGLED_MODE_A			0
+#define AXP20X_CHGLED_MODE_B			1
+#define AXP20X_CHGLED_MODE(_mode)		(_mode << 4)
+
+struct axp20x_led {
+	char			name[LED_MAX_NAME_SIZE];
+	struct led_classdev	cdev;
+	struct mutex		lock;
+	u8			mode : 1;
+	u8			ctrl : 1;
+	u8			ctrl_inverted : 1;
+	struct axp20x_dev	*axp20x;
+};
+
+static inline struct axp20x_led *to_axp20x_led(struct led_classdev *cdev)
+{
+	return container_of(cdev, struct axp20x_led, cdev);
+}
+
+static int axp20x_led_setup(struct axp20x_led *priv)
+{
+	int ret;
+	u8 val;
+
+	/* Invert the logic, if necessary */
+	val = priv->ctrl ^ priv->ctrl_inverted;
+
+	mutex_lock(&priv->lock);
+	ret = regmap_update_bits(priv->axp20x->regmap, AXP20X_CHGLED_CTRL_REG,
+				 AXP20X_CHGLED_CTRL_MASK,
+				 AXP20X_CHGLED_CTRL(val));
+	if (ret < 0)
+		goto out;
+
+	ret = regmap_update_bits(priv->axp20x->regmap, AXP20X_CHGLED_MODE_REG,
+				 AXP20X_CHGLED_MODE_MASK,
+				 AXP20X_CHGLED_MODE(priv->mode));
+out:
+	mutex_unlock(&priv->lock);
+	return ret;
+}
+
+static ssize_t control_show(struct device *dev, struct device_attribute *attr,
+			    char *buf)
+{
+	struct led_classdev *cdev = dev_get_drvdata(dev);
+	struct axp20x_led *priv = to_axp20x_led(cdev);
+
+	return sprintf(buf, "%u\n", priv->ctrl);
+}
+
+static ssize_t control_store(struct device *dev, struct device_attribute *attr,
+			     const char *buf, size_t size)
+{
+	struct led_classdev *cdev = dev_get_drvdata(dev);
+	struct axp20x_led *priv = to_axp20x_led(cdev);
+	unsigned long val;
+	int ret;
+
+	ret = kstrtoul(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	/**
+	 * Supported values are:
+	 *   - 0 : Manual control
+	 *   - 1 : Charger control
+	 */
+	if (val > 1)
+		return -EINVAL;
+
+	priv->ctrl = val;
+
+	return axp20x_led_setup(priv) ? : size;
+}
+static DEVICE_ATTR_RW(control);
+
+static ssize_t mode_show(struct device *dev, struct device_attribute *attr,
+			 char *buf)
+{
+	struct led_classdev *cdev = dev_get_drvdata(dev);
+	struct axp20x_led *priv = to_axp20x_led(cdev);
+
+	return sprintf(buf, "%u\n", priv->mode);
+}
+
+static ssize_t mode_store(struct device *dev, struct device_attribute *attr,
+			  const char *buf, size_t size)
+{
+	struct led_classdev *cdev = dev_get_drvdata(dev);
+	struct axp20x_led *priv = to_axp20x_led(cdev);
+	unsigned long val;
+	int ret;
+
+	ret = kstrtoul(buf, 0, &val);
+	if (ret)
+		return ret;
+	/**
+	 * Supported values are:
+	 *   - 0 : Mode A
+	 *   - 1 : Mode B
+	 */
+	if (val > 1)
+		return -EINVAL;
+
+	priv->mode = val;
+
+	return axp20x_led_setup(priv) ? : size;
+}
+static DEVICE_ATTR_RW(mode);
+
+static struct attribute *axp20x_led_attrs[] = {
+	&dev_attr_control.attr,
+	&dev_attr_mode.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(axp20x_led);
+
+enum led_brightness axp20x_led_brightness_get(struct led_classdev *cdev)
+{
+	struct axp20x_led *priv = to_axp20x_led(cdev);
+	u32 val;
+	int ret;
+
+	mutex_lock(&priv->lock);
+	ret = regmap_read(priv->axp20x->regmap, AXP20X_CHGLED_CTRL_REG, &val);
+	mutex_unlock(&priv->lock);
+	if (ret < 0)
+		return LED_OFF;
+
+	return (val & AXP20X_CHGLED_FUNC_FULL) ? LED_FULL : LED_OFF;
+}
+
+static int axp20x_led_brightness_set_blocking(struct led_classdev *cdev,
+					      enum led_brightness brightness)
+{
+	struct axp20x_led *priv = to_axp20x_led(cdev);
+	int ret = 0;
+
+	mutex_lock(&priv->lock);
+	ret = regmap_update_bits(priv->axp20x->regmap,
+				 AXP20X_CHGLED_CTRL_REG,
+				 AXP20X_CHGLED_FUNC_MASK,
+				 (brightness) ?
+				 AXP20X_CHGLED_FUNC_FULL :
+				 AXP20X_CHGLED_FUNC_OFF);
+	mutex_unlock(&priv->lock);
+
+	return ret;
+}
+
+static int axp20x_led_parse_dt(struct axp20x_led *priv, struct device_node *np)
+{
+	const char *str;
+	u8 value;
+	int ret = 0;
+
+	str = of_get_property(np, "label", NULL);
+	if (!str)
+		snprintf(priv->name, sizeof(priv->name), "axp20x::");
+	else
+		snprintf(priv->name, sizeof(priv->name), "axp20x:%s", str);
+	priv->cdev.name = priv->name;
+
+	priv->cdev.default_trigger = of_get_property(np,
+						     "linux,default-trigger",
+						     NULL);
+
+	if (!of_property_read_u8(np, "x-powers,charger-mode", &value)) {
+		priv->ctrl = AXP20X_CHGLED_CTRL_CHARGER;
+		priv->mode = (value < 2) ? value : 0;
+	} else {
+		priv->ctrl = AXP20X_CHGLED_CTRL_MANUAL;
+	}
+
+	str = of_get_property(np, "default-state", NULL);
+	if (str) {
+		if (!strcmp(str, "keep")) {
+			ret = axp20x_led_brightness_get(&priv->cdev);
+			if (ret < 0)
+				return ret;
+			priv->cdev.brightness = ret;
+		} else if (!strcmp(str, "on")) {
+			ret = axp20x_led_brightness_set_blocking(&priv->cdev,
+								 LED_FULL);
+		} else  {
+			ret = axp20x_led_brightness_set_blocking(&priv->cdev,
+								 LED_OFF);
+		}
+	}
+
+	return ret;
+}
+
+static const struct of_device_id axp20x_led_of_match[] = {
+	{ .compatible = "x-powers,axp20x-led" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, axp20x_led_of_match);
+
+static int axp20x_led_probe(struct platform_device *pdev)
+{
+	struct axp20x_led *priv;
+	int ret;
+
+	if (!of_device_is_available(pdev->dev.of_node))
+		return -ENODEV;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(struct axp20x_led),
+			    GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->axp20x = dev_get_drvdata(pdev->dev.parent);
+	if (!priv->axp20x) {
+		dev_err(&pdev->dev, "Failed to get parent data\n");
+		return -ENXIO;
+	}
+
+	mutex_init(&priv->lock);
+
+	priv->cdev.brightness_set_blocking = axp20x_led_brightness_set_blocking;
+	priv->cdev.brightness_get = axp20x_led_brightness_get;
+	priv->cdev.groups = axp20x_led_groups;
+
+	ret = axp20x_led_parse_dt(priv, pdev->dev.of_node);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to set parameters\n");
+		return ret;
+	}
+
+	/**
+	 * For some reason in AXP209 the bit that controls CHGLED is with
+	 * inverted logic compared to all other PMICs.
+	 * If the PMIC is actually AXP209, set inverted flag and later use it
+	 * when configuring the LED.
+	 */
+	if (priv->axp20x->variant == AXP209_ID)
+		priv->ctrl_inverted = 1;
+
+	ret =  axp20x_led_setup(priv);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to configure led");
+		return ret;
+	}
+
+	return devm_led_classdev_register(&pdev->dev, &priv->cdev);
+}
+
+static struct platform_driver axp20x_led_driver = {
+	.driver = {
+		.name	= "axp20x-led",
+		.of_match_table = of_match_ptr(axp20x_led_of_match),
+	},
+	.probe = axp20x_led_probe,
+};
+
+module_platform_driver(axp20x_led_driver);
+
+MODULE_AUTHOR("Stefan Mavrodiev <stefan@olimex.com");
+MODULE_DESCRIPTION("X-Powers PMIC CHGLED driver");
+MODULE_LICENSE("GPL");
-- 
2.17.1


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

* [PATCH v2 1/8] leds: Add support for AXP20X CHGLED
@ 2019-02-15 11:50   ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

Most of AXP20x PMIC chips have built-in battery charger with LED indicator.
The LED can be controlled ether by the charger or manually by a register.

The default is (except for AXP209) manual control, which makes this LED
useless, since there is no device driver.

The driver rely on AXP20X MFD driver.

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 drivers/leds/Kconfig       |  10 ++
 drivers/leds/Makefile      |   1 +
 drivers/leds/leds-axp20x.c | 291 +++++++++++++++++++++++++++++++++++++
 3 files changed, 302 insertions(+)
 create mode 100644 drivers/leds/leds-axp20x.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index a72f97fca57b..82dce9063d41 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -766,6 +766,16 @@ config LEDS_NIC78BX
 	  To compile this driver as a module, choose M here: the module
 	  will be called leds-nic78bx.
 
+config LEDS_AXP20X
+	tristate "LED support for X-Powers PMICs"
+	depends on MFD_AXP20X
+	help
+	  This option enables support for CHGLED found on most of X-Powers
+	  PMICs.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called leds-axp20x.
+
 comment "LED Triggers"
 source "drivers/leds/trigger/Kconfig"
 
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 4c1b0054f379..d3fb76e119d8 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -79,6 +79,7 @@ obj-$(CONFIG_LEDS_MT6323)		+= leds-mt6323.o
 obj-$(CONFIG_LEDS_LM3692X)		+= leds-lm3692x.o
 obj-$(CONFIG_LEDS_SC27XX_BLTC)		+= leds-sc27xx-bltc.o
 obj-$(CONFIG_LEDS_LM3601X)		+= leds-lm3601x.o
+obj-$(CONFIG_LEDS_AXP20X)		+= leds-axp20x.o
 
 # LED SPI Drivers
 obj-$(CONFIG_LEDS_CR0014114)		+= leds-cr0014114.o
diff --git a/drivers/leds/leds-axp20x.c b/drivers/leds/leds-axp20x.c
new file mode 100644
index 000000000000..2d5ae1c085f0
--- /dev/null
+++ b/drivers/leds/leds-axp20x.c
@@ -0,0 +1,291 @@
+// SPDX-License-Identifier: GPL-2.0+
+//
+// Copyright 2019 Stefan Mavrodiev <stefan@olimex.com>
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/leds.h>
+#include <linux/mfd/axp20x.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <uapi/linux/uleds.h>
+
+
+#define AXP20X_CHGLED_CTRL_REG		AXP20X_OFF_CTRL
+#define AXP20X_CHGLED_FUNC_MASK			GENMASK(5, 4)
+#define AXP20X_CHGLED_FUNC_OFF			(0 << 4)
+#define AXP20X_CHGLED_FUNC_1HZ			(1 << 4)
+#define AXP20X_CHGLED_FUNC_4HZ			(2 << 4)
+#define AXP20X_CHGLED_FUNC_FULL			(3 << 4)
+#define AXP20X_CHGLED_CTRL_MASK			BIT(3)
+#define AXP20X_CHGLED_CTRL_MANUAL		0
+#define AXP20X_CHGLED_CTRL_CHARGER		1
+#define AXP20X_CHGLED_CTRL(_ctrl)		(_ctrl << 3)
+
+#define AXP20X_CHGLED_MODE_REG		AXP20X_CHRG_CTRL2
+#define AXP20X_CHGLED_MODE_MASK			BIT(4)
+#define AXP20X_CHGLED_MODE_A			0
+#define AXP20X_CHGLED_MODE_B			1
+#define AXP20X_CHGLED_MODE(_mode)		(_mode << 4)
+
+struct axp20x_led {
+	char			name[LED_MAX_NAME_SIZE];
+	struct led_classdev	cdev;
+	struct mutex		lock;
+	u8			mode : 1;
+	u8			ctrl : 1;
+	u8			ctrl_inverted : 1;
+	struct axp20x_dev	*axp20x;
+};
+
+static inline struct axp20x_led *to_axp20x_led(struct led_classdev *cdev)
+{
+	return container_of(cdev, struct axp20x_led, cdev);
+}
+
+static int axp20x_led_setup(struct axp20x_led *priv)
+{
+	int ret;
+	u8 val;
+
+	/* Invert the logic, if necessary */
+	val = priv->ctrl ^ priv->ctrl_inverted;
+
+	mutex_lock(&priv->lock);
+	ret = regmap_update_bits(priv->axp20x->regmap, AXP20X_CHGLED_CTRL_REG,
+				 AXP20X_CHGLED_CTRL_MASK,
+				 AXP20X_CHGLED_CTRL(val));
+	if (ret < 0)
+		goto out;
+
+	ret = regmap_update_bits(priv->axp20x->regmap, AXP20X_CHGLED_MODE_REG,
+				 AXP20X_CHGLED_MODE_MASK,
+				 AXP20X_CHGLED_MODE(priv->mode));
+out:
+	mutex_unlock(&priv->lock);
+	return ret;
+}
+
+static ssize_t control_show(struct device *dev, struct device_attribute *attr,
+			    char *buf)
+{
+	struct led_classdev *cdev = dev_get_drvdata(dev);
+	struct axp20x_led *priv = to_axp20x_led(cdev);
+
+	return sprintf(buf, "%u\n", priv->ctrl);
+}
+
+static ssize_t control_store(struct device *dev, struct device_attribute *attr,
+			     const char *buf, size_t size)
+{
+	struct led_classdev *cdev = dev_get_drvdata(dev);
+	struct axp20x_led *priv = to_axp20x_led(cdev);
+	unsigned long val;
+	int ret;
+
+	ret = kstrtoul(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	/**
+	 * Supported values are:
+	 *   - 0 : Manual control
+	 *   - 1 : Charger control
+	 */
+	if (val > 1)
+		return -EINVAL;
+
+	priv->ctrl = val;
+
+	return axp20x_led_setup(priv) ? : size;
+}
+static DEVICE_ATTR_RW(control);
+
+static ssize_t mode_show(struct device *dev, struct device_attribute *attr,
+			 char *buf)
+{
+	struct led_classdev *cdev = dev_get_drvdata(dev);
+	struct axp20x_led *priv = to_axp20x_led(cdev);
+
+	return sprintf(buf, "%u\n", priv->mode);
+}
+
+static ssize_t mode_store(struct device *dev, struct device_attribute *attr,
+			  const char *buf, size_t size)
+{
+	struct led_classdev *cdev = dev_get_drvdata(dev);
+	struct axp20x_led *priv = to_axp20x_led(cdev);
+	unsigned long val;
+	int ret;
+
+	ret = kstrtoul(buf, 0, &val);
+	if (ret)
+		return ret;
+	/**
+	 * Supported values are:
+	 *   - 0 : Mode A
+	 *   - 1 : Mode B
+	 */
+	if (val > 1)
+		return -EINVAL;
+
+	priv->mode = val;
+
+	return axp20x_led_setup(priv) ? : size;
+}
+static DEVICE_ATTR_RW(mode);
+
+static struct attribute *axp20x_led_attrs[] = {
+	&dev_attr_control.attr,
+	&dev_attr_mode.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(axp20x_led);
+
+enum led_brightness axp20x_led_brightness_get(struct led_classdev *cdev)
+{
+	struct axp20x_led *priv = to_axp20x_led(cdev);
+	u32 val;
+	int ret;
+
+	mutex_lock(&priv->lock);
+	ret = regmap_read(priv->axp20x->regmap, AXP20X_CHGLED_CTRL_REG, &val);
+	mutex_unlock(&priv->lock);
+	if (ret < 0)
+		return LED_OFF;
+
+	return (val & AXP20X_CHGLED_FUNC_FULL) ? LED_FULL : LED_OFF;
+}
+
+static int axp20x_led_brightness_set_blocking(struct led_classdev *cdev,
+					      enum led_brightness brightness)
+{
+	struct axp20x_led *priv = to_axp20x_led(cdev);
+	int ret = 0;
+
+	mutex_lock(&priv->lock);
+	ret = regmap_update_bits(priv->axp20x->regmap,
+				 AXP20X_CHGLED_CTRL_REG,
+				 AXP20X_CHGLED_FUNC_MASK,
+				 (brightness) ?
+				 AXP20X_CHGLED_FUNC_FULL :
+				 AXP20X_CHGLED_FUNC_OFF);
+	mutex_unlock(&priv->lock);
+
+	return ret;
+}
+
+static int axp20x_led_parse_dt(struct axp20x_led *priv, struct device_node *np)
+{
+	const char *str;
+	u8 value;
+	int ret = 0;
+
+	str = of_get_property(np, "label", NULL);
+	if (!str)
+		snprintf(priv->name, sizeof(priv->name), "axp20x::");
+	else
+		snprintf(priv->name, sizeof(priv->name), "axp20x:%s", str);
+	priv->cdev.name = priv->name;
+
+	priv->cdev.default_trigger = of_get_property(np,
+						     "linux,default-trigger",
+						     NULL);
+
+	if (!of_property_read_u8(np, "x-powers,charger-mode", &value)) {
+		priv->ctrl = AXP20X_CHGLED_CTRL_CHARGER;
+		priv->mode = (value < 2) ? value : 0;
+	} else {
+		priv->ctrl = AXP20X_CHGLED_CTRL_MANUAL;
+	}
+
+	str = of_get_property(np, "default-state", NULL);
+	if (str) {
+		if (!strcmp(str, "keep")) {
+			ret = axp20x_led_brightness_get(&priv->cdev);
+			if (ret < 0)
+				return ret;
+			priv->cdev.brightness = ret;
+		} else if (!strcmp(str, "on")) {
+			ret = axp20x_led_brightness_set_blocking(&priv->cdev,
+								 LED_FULL);
+		} else  {
+			ret = axp20x_led_brightness_set_blocking(&priv->cdev,
+								 LED_OFF);
+		}
+	}
+
+	return ret;
+}
+
+static const struct of_device_id axp20x_led_of_match[] = {
+	{ .compatible = "x-powers,axp20x-led" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, axp20x_led_of_match);
+
+static int axp20x_led_probe(struct platform_device *pdev)
+{
+	struct axp20x_led *priv;
+	int ret;
+
+	if (!of_device_is_available(pdev->dev.of_node))
+		return -ENODEV;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(struct axp20x_led),
+			    GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->axp20x = dev_get_drvdata(pdev->dev.parent);
+	if (!priv->axp20x) {
+		dev_err(&pdev->dev, "Failed to get parent data\n");
+		return -ENXIO;
+	}
+
+	mutex_init(&priv->lock);
+
+	priv->cdev.brightness_set_blocking = axp20x_led_brightness_set_blocking;
+	priv->cdev.brightness_get = axp20x_led_brightness_get;
+	priv->cdev.groups = axp20x_led_groups;
+
+	ret = axp20x_led_parse_dt(priv, pdev->dev.of_node);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to set parameters\n");
+		return ret;
+	}
+
+	/**
+	 * For some reason in AXP209 the bit that controls CHGLED is with
+	 * inverted logic compared to all other PMICs.
+	 * If the PMIC is actually AXP209, set inverted flag and later use it
+	 * when configuring the LED.
+	 */
+	if (priv->axp20x->variant == AXP209_ID)
+		priv->ctrl_inverted = 1;
+
+	ret =  axp20x_led_setup(priv);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to configure led");
+		return ret;
+	}
+
+	return devm_led_classdev_register(&pdev->dev, &priv->cdev);
+}
+
+static struct platform_driver axp20x_led_driver = {
+	.driver = {
+		.name	= "axp20x-led",
+		.of_match_table = of_match_ptr(axp20x_led_of_match),
+	},
+	.probe = axp20x_led_probe,
+};
+
+module_platform_driver(axp20x_led_driver);
+
+MODULE_AUTHOR("Stefan Mavrodiev <stefan@olimex.com");
+MODULE_DESCRIPTION("X-Powers PMIC CHGLED driver");
+MODULE_LICENSE("GPL");
-- 
2.17.1


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

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

* [PATCH v2 2/8] mfd: axp20x: Add axp20x-led cell
  2019-02-15 11:50 ` Stefan Mavrodiev
  (?)
@ 2019-02-15 11:50   ` Stefan Mavrodiev
  -1 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

Add axp20x-led cell for AXP20x, AXP221, AXP223, AXP228, AXP803, AXP809
and AXP813.

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 drivers/mfd/axp20x.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 3c97f2c0fdfe..e6ab078f0462 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -610,6 +610,9 @@ static const struct mfd_cell axp20x_cells[] = {
 		.of_compatible	= "x-powers,axp202-usb-power-supply",
 		.num_resources	= ARRAY_SIZE(axp20x_usb_power_supply_resources),
 		.resources	= axp20x_usb_power_supply_resources,
+	}, {
+		.name		= "axp20x-led",
+		.of_compatible	= "x-powers,axp20x-led",
 	},
 };
 
@@ -636,6 +639,9 @@ static const struct mfd_cell axp221_cells[] = {
 		.of_compatible	= "x-powers,axp221-usb-power-supply",
 		.num_resources	= ARRAY_SIZE(axp22x_usb_power_supply_resources),
 		.resources	= axp22x_usb_power_supply_resources,
+	}, {
+		.name		= "axp20x-led",
+		.of_compatible	= "x-powers,axp20x-led",
 	},
 };
 
@@ -662,6 +668,9 @@ static const struct mfd_cell axp223_cells[] = {
 		.of_compatible	= "x-powers,axp223-usb-power-supply",
 		.num_resources	= ARRAY_SIZE(axp22x_usb_power_supply_resources),
 		.resources	= axp22x_usb_power_supply_resources,
+	}, {
+		.name		= "axp20x-led",
+		.of_compatible	= "x-powers,axp20x-led",
 	},
 };
 
@@ -719,6 +728,9 @@ static const struct mfd_cell axp288_cells[] = {
 		.resources	= axp288_power_button_resources,
 	}, {
 		.name		= "axp288_pmic_acpi",
+	}, {
+		.name		= "axp20x-led",
+		.of_compatible	= "x-powers,axp20x-led",
 	},
 };
 
@@ -741,8 +753,12 @@ static const struct mfd_cell axp803_cells[] = {
 		.of_compatible	= "x-powers,axp813-ac-power-supply",
 		.num_resources	= ARRAY_SIZE(axp20x_ac_power_supply_resources),
 		.resources	= axp20x_ac_power_supply_resources,
+	}, {
+		.name		= "axp20x-regulator"
+	}, {
+		.name		= "axp20x-led",
+		.of_compatible	= "x-powers,axp20x-led",
 	},
-	{	.name		= "axp20x-regulator" },
 };
 
 static const struct mfd_cell axp806_self_working_cells[] = {
@@ -769,6 +785,9 @@ static const struct mfd_cell axp809_cells[] = {
 	}, {
 		.id		= 1,
 		.name		= "axp20x-regulator",
+	}, {
+		.name		= "axp20x-led",
+		.of_compatible	= "x-powers,axp20x-led",
 	},
 };
 
@@ -793,6 +812,9 @@ static const struct mfd_cell axp813_cells[] = {
 		.of_compatible	= "x-powers,axp813-ac-power-supply",
 		.num_resources	= ARRAY_SIZE(axp20x_ac_power_supply_resources),
 		.resources	= axp20x_ac_power_supply_resources,
+	}, {
+		.name		= "axp20x-led",
+		.of_compatible	= "x-powers,axp20x-led",
 	},
 };
 
-- 
2.17.1

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

* [PATCH v2 2/8] mfd: axp20x: Add axp20x-led cell
@ 2019-02-15 11:50   ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

Add axp20x-led cell for AXP20x, AXP221, AXP223, AXP228, AXP803, AXP809
and AXP813.

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 drivers/mfd/axp20x.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 3c97f2c0fdfe..e6ab078f0462 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -610,6 +610,9 @@ static const struct mfd_cell axp20x_cells[] = {
 		.of_compatible	= "x-powers,axp202-usb-power-supply",
 		.num_resources	= ARRAY_SIZE(axp20x_usb_power_supply_resources),
 		.resources	= axp20x_usb_power_supply_resources,
+	}, {
+		.name		= "axp20x-led",
+		.of_compatible	= "x-powers,axp20x-led",
 	},
 };
 
@@ -636,6 +639,9 @@ static const struct mfd_cell axp221_cells[] = {
 		.of_compatible	= "x-powers,axp221-usb-power-supply",
 		.num_resources	= ARRAY_SIZE(axp22x_usb_power_supply_resources),
 		.resources	= axp22x_usb_power_supply_resources,
+	}, {
+		.name		= "axp20x-led",
+		.of_compatible	= "x-powers,axp20x-led",
 	},
 };
 
@@ -662,6 +668,9 @@ static const struct mfd_cell axp223_cells[] = {
 		.of_compatible	= "x-powers,axp223-usb-power-supply",
 		.num_resources	= ARRAY_SIZE(axp22x_usb_power_supply_resources),
 		.resources	= axp22x_usb_power_supply_resources,
+	}, {
+		.name		= "axp20x-led",
+		.of_compatible	= "x-powers,axp20x-led",
 	},
 };
 
@@ -719,6 +728,9 @@ static const struct mfd_cell axp288_cells[] = {
 		.resources	= axp288_power_button_resources,
 	}, {
 		.name		= "axp288_pmic_acpi",
+	}, {
+		.name		= "axp20x-led",
+		.of_compatible	= "x-powers,axp20x-led",
 	},
 };
 
@@ -741,8 +753,12 @@ static const struct mfd_cell axp803_cells[] = {
 		.of_compatible	= "x-powers,axp813-ac-power-supply",
 		.num_resources	= ARRAY_SIZE(axp20x_ac_power_supply_resources),
 		.resources	= axp20x_ac_power_supply_resources,
+	}, {
+		.name		= "axp20x-regulator"
+	}, {
+		.name		= "axp20x-led",
+		.of_compatible	= "x-powers,axp20x-led",
 	},
-	{	.name		= "axp20x-regulator" },
 };
 
 static const struct mfd_cell axp806_self_working_cells[] = {
@@ -769,6 +785,9 @@ static const struct mfd_cell axp809_cells[] = {
 	}, {
 		.id		= 1,
 		.name		= "axp20x-regulator",
+	}, {
+		.name		= "axp20x-led",
+		.of_compatible	= "x-powers,axp20x-led",
 	},
 };
 
@@ -793,6 +812,9 @@ static const struct mfd_cell axp813_cells[] = {
 		.of_compatible	= "x-powers,axp813-ac-power-supply",
 		.num_resources	= ARRAY_SIZE(axp20x_ac_power_supply_resources),
 		.resources	= axp20x_ac_power_supply_resources,
+	}, {
+		.name		= "axp20x-led",
+		.of_compatible	= "x-powers,axp20x-led",
 	},
 };
 
-- 
2.17.1


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

* [PATCH v2 2/8] mfd: axp20x: Add axp20x-led cell
@ 2019-02-15 11:50   ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

Add axp20x-led cell for AXP20x, AXP221, AXP223, AXP228, AXP803, AXP809
and AXP813.

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 drivers/mfd/axp20x.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 3c97f2c0fdfe..e6ab078f0462 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -610,6 +610,9 @@ static const struct mfd_cell axp20x_cells[] = {
 		.of_compatible	= "x-powers,axp202-usb-power-supply",
 		.num_resources	= ARRAY_SIZE(axp20x_usb_power_supply_resources),
 		.resources	= axp20x_usb_power_supply_resources,
+	}, {
+		.name		= "axp20x-led",
+		.of_compatible	= "x-powers,axp20x-led",
 	},
 };
 
@@ -636,6 +639,9 @@ static const struct mfd_cell axp221_cells[] = {
 		.of_compatible	= "x-powers,axp221-usb-power-supply",
 		.num_resources	= ARRAY_SIZE(axp22x_usb_power_supply_resources),
 		.resources	= axp22x_usb_power_supply_resources,
+	}, {
+		.name		= "axp20x-led",
+		.of_compatible	= "x-powers,axp20x-led",
 	},
 };
 
@@ -662,6 +668,9 @@ static const struct mfd_cell axp223_cells[] = {
 		.of_compatible	= "x-powers,axp223-usb-power-supply",
 		.num_resources	= ARRAY_SIZE(axp22x_usb_power_supply_resources),
 		.resources	= axp22x_usb_power_supply_resources,
+	}, {
+		.name		= "axp20x-led",
+		.of_compatible	= "x-powers,axp20x-led",
 	},
 };
 
@@ -719,6 +728,9 @@ static const struct mfd_cell axp288_cells[] = {
 		.resources	= axp288_power_button_resources,
 	}, {
 		.name		= "axp288_pmic_acpi",
+	}, {
+		.name		= "axp20x-led",
+		.of_compatible	= "x-powers,axp20x-led",
 	},
 };
 
@@ -741,8 +753,12 @@ static const struct mfd_cell axp803_cells[] = {
 		.of_compatible	= "x-powers,axp813-ac-power-supply",
 		.num_resources	= ARRAY_SIZE(axp20x_ac_power_supply_resources),
 		.resources	= axp20x_ac_power_supply_resources,
+	}, {
+		.name		= "axp20x-regulator"
+	}, {
+		.name		= "axp20x-led",
+		.of_compatible	= "x-powers,axp20x-led",
 	},
-	{	.name		= "axp20x-regulator" },
 };
 
 static const struct mfd_cell axp806_self_working_cells[] = {
@@ -769,6 +785,9 @@ static const struct mfd_cell axp809_cells[] = {
 	}, {
 		.id		= 1,
 		.name		= "axp20x-regulator",
+	}, {
+		.name		= "axp20x-led",
+		.of_compatible	= "x-powers,axp20x-led",
 	},
 };
 
@@ -793,6 +812,9 @@ static const struct mfd_cell axp813_cells[] = {
 		.of_compatible	= "x-powers,axp813-ac-power-supply",
 		.num_resources	= ARRAY_SIZE(axp20x_ac_power_supply_resources),
 		.resources	= axp20x_ac_power_supply_resources,
+	}, {
+		.name		= "axp20x-led",
+		.of_compatible	= "x-powers,axp20x-led",
 	},
 };
 
-- 
2.17.1


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

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

* [PATCH v2 3/8] dt-bindings: leds: Add binding for axp20x-led device driver
  2019-02-15 11:50 ` Stefan Mavrodiev
  (?)
@ 2019-02-15 11:50   ` Stefan Mavrodiev
  -1 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

This adds the devicetree bindings for charge led indicator found
on most of X-Powers AXP20X PMICs.

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 .../devicetree/bindings/leds/leds-axp20x.txt  | 74 +++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-axp20x.txt

diff --git a/Documentation/devicetree/bindings/leds/leds-axp20x.txt b/Documentation/devicetree/bindings/leds/leds-axp20x.txt
new file mode 100644
index 000000000000..5a83ad06796d
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-axp20x.txt
@@ -0,0 +1,74 @@
+Device Tree Bindings for LED support on X-Powers PMIC
+
+Most of the X-Powers PMICs have integrated battery charger with LED indicator.
+The output is open-drain, so the state is either high-Z or output-low. The
+driver is a subnode of AXP20X MFD driver, since it uses shared bus with all
+other cells.
+The LED can be controlled either manually or automatically. Then in automatic
+(controlled by the charger) there are two indication modes:
+
+Mode-A
+======
+- output-low:		Charging
+- high-Z		Not charging
+- 1Hz flashing:		Abnormal alarm
+- 4Hz flashing		Overvoltage alarm
+
+Mode-B
+======
+- output-low:		Battery full
+- high-Z		Not charging
+- 1Hz flashing:		Charging
+- 4Hz flashing		Overvoltage or abnormal alarm
+
+The control and the mode can be changed from sysfs.
+
+For AXP20X MFD bindings see:
+Documentation/devicetree/bindings/mfd/axp20x.txt
+
+Required properties:
+- compatible : Must be "x-powers,axp20x-led"
+
+Supported common LED properties, see ./common.txt for more informationn
+- label : See Documentation/devicetree/bindings/leds/common.txt
+- linux,default-trigger : See Documentation/devicetree/bindings/leds/common.txt
+- default-state: See Documentation/devicetree/bindings/leds/common.txt
+
+Optional properties:
+- x-powers,charger-mode: 0 for Mode-A, 1 for Mode-B
+			 If omitted, then the control is set to manual mode.
+			 On invalid value, Mode-A is used.
+
+
+Example:
+
+	axp803: pmic@3a3 {
+		compatible = "x-powers,axp803";
+
+		...
+
+		led@0 {
+			compatible = "x-powers,axp20x-led";
+			status = "okay";
+
+			label = "axp20x:yellow:chgled";
+			linux,default-trigger = "timer";
+			default-state = "on";
+		};
+	};
+
+or
+
+	axp803: pmic@3a3 {
+		compatible = "x-powers,axp803";
+
+		...
+
+		led@0 {
+			compatible = "x-powers,axp20x-led";
+			status = "okay";
+
+			label = "axp20x:yellow:chgled";
+			x-powers,charger-mode = <1>;
+		};
+	};
-- 
2.17.1

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

* [PATCH v2 3/8] dt-bindings: leds: Add binding for axp20x-led device driver
@ 2019-02-15 11:50   ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

This adds the devicetree bindings for charge led indicator found
on most of X-Powers AXP20X PMICs.

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 .../devicetree/bindings/leds/leds-axp20x.txt  | 74 +++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-axp20x.txt

diff --git a/Documentation/devicetree/bindings/leds/leds-axp20x.txt b/Documentation/devicetree/bindings/leds/leds-axp20x.txt
new file mode 100644
index 000000000000..5a83ad06796d
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-axp20x.txt
@@ -0,0 +1,74 @@
+Device Tree Bindings for LED support on X-Powers PMIC
+
+Most of the X-Powers PMICs have integrated battery charger with LED indicator.
+The output is open-drain, so the state is either high-Z or output-low. The
+driver is a subnode of AXP20X MFD driver, since it uses shared bus with all
+other cells.
+The LED can be controlled either manually or automatically. Then in automatic
+(controlled by the charger) there are two indication modes:
+
+Mode-A
+======
+- output-low:		Charging
+- high-Z		Not charging
+- 1Hz flashing:		Abnormal alarm
+- 4Hz flashing		Overvoltage alarm
+
+Mode-B
+======
+- output-low:		Battery full
+- high-Z		Not charging
+- 1Hz flashing:		Charging
+- 4Hz flashing		Overvoltage or abnormal alarm
+
+The control and the mode can be changed from sysfs.
+
+For AXP20X MFD bindings see:
+Documentation/devicetree/bindings/mfd/axp20x.txt
+
+Required properties:
+- compatible : Must be "x-powers,axp20x-led"
+
+Supported common LED properties, see ./common.txt for more informationn
+- label : See Documentation/devicetree/bindings/leds/common.txt
+- linux,default-trigger : See Documentation/devicetree/bindings/leds/common.txt
+- default-state: See Documentation/devicetree/bindings/leds/common.txt
+
+Optional properties:
+- x-powers,charger-mode: 0 for Mode-A, 1 for Mode-B
+			 If omitted, then the control is set to manual mode.
+			 On invalid value, Mode-A is used.
+
+
+Example:
+
+	axp803: pmic@3a3 {
+		compatible = "x-powers,axp803";
+
+		...
+
+		led@0 {
+			compatible = "x-powers,axp20x-led";
+			status = "okay";
+
+			label = "axp20x:yellow:chgled";
+			linux,default-trigger = "timer";
+			default-state = "on";
+		};
+	};
+
+or
+
+	axp803: pmic@3a3 {
+		compatible = "x-powers,axp803";
+
+		...
+
+		led@0 {
+			compatible = "x-powers,axp20x-led";
+			status = "okay";
+
+			label = "axp20x:yellow:chgled";
+			x-powers,charger-mode = <1>;
+		};
+	};
-- 
2.17.1


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

* [PATCH v2 3/8] dt-bindings: leds: Add binding for axp20x-led device driver
@ 2019-02-15 11:50   ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

This adds the devicetree bindings for charge led indicator found
on most of X-Powers AXP20X PMICs.

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 .../devicetree/bindings/leds/leds-axp20x.txt  | 74 +++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-axp20x.txt

diff --git a/Documentation/devicetree/bindings/leds/leds-axp20x.txt b/Documentation/devicetree/bindings/leds/leds-axp20x.txt
new file mode 100644
index 000000000000..5a83ad06796d
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-axp20x.txt
@@ -0,0 +1,74 @@
+Device Tree Bindings for LED support on X-Powers PMIC
+
+Most of the X-Powers PMICs have integrated battery charger with LED indicator.
+The output is open-drain, so the state is either high-Z or output-low. The
+driver is a subnode of AXP20X MFD driver, since it uses shared bus with all
+other cells.
+The LED can be controlled either manually or automatically. Then in automatic
+(controlled by the charger) there are two indication modes:
+
+Mode-A
+======
+- output-low:		Charging
+- high-Z		Not charging
+- 1Hz flashing:		Abnormal alarm
+- 4Hz flashing		Overvoltage alarm
+
+Mode-B
+======
+- output-low:		Battery full
+- high-Z		Not charging
+- 1Hz flashing:		Charging
+- 4Hz flashing		Overvoltage or abnormal alarm
+
+The control and the mode can be changed from sysfs.
+
+For AXP20X MFD bindings see:
+Documentation/devicetree/bindings/mfd/axp20x.txt
+
+Required properties:
+- compatible : Must be "x-powers,axp20x-led"
+
+Supported common LED properties, see ./common.txt for more informationn
+- label : See Documentation/devicetree/bindings/leds/common.txt
+- linux,default-trigger : See Documentation/devicetree/bindings/leds/common.txt
+- default-state: See Documentation/devicetree/bindings/leds/common.txt
+
+Optional properties:
+- x-powers,charger-mode: 0 for Mode-A, 1 for Mode-B
+			 If omitted, then the control is set to manual mode.
+			 On invalid value, Mode-A is used.
+
+
+Example:
+
+	axp803: pmic@3a3 {
+		compatible = "x-powers,axp803";
+
+		...
+
+		led@0 {
+			compatible = "x-powers,axp20x-led";
+			status = "okay";
+
+			label = "axp20x:yellow:chgled";
+			linux,default-trigger = "timer";
+			default-state = "on";
+		};
+	};
+
+or
+
+	axp803: pmic@3a3 {
+		compatible = "x-powers,axp803";
+
+		...
+
+		led@0 {
+			compatible = "x-powers,axp20x-led";
+			status = "okay";
+
+			label = "axp20x:yellow:chgled";
+			x-powers,charger-mode = <1>;
+		};
+	};
-- 
2.17.1


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

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

* [PATCH v2 4/8] arm64: dts: allwinner: axp803: add charge led node
  2019-02-15 11:50 ` Stefan Mavrodiev
  (?)
@ 2019-02-15 11:50   ` Stefan Mavrodiev
  -1 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

Add dt node for axp20x-led driver controlling CHGLED.
Default status is disabled, since it may be not used.

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 arch/arm64/boot/dts/allwinner/axp803.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/axp803.dtsi b/arch/arm64/boot/dts/allwinner/axp803.dtsi
index c3a618e1279a..af4898602b34 100644
--- a/arch/arm64/boot/dts/allwinner/axp803.dtsi
+++ b/arch/arm64/boot/dts/allwinner/axp803.dtsi
@@ -76,6 +76,11 @@
 		};
 	};
 
+	axp_led: led {
+		compatible = "x-powers,axp20x-led";
+		status = "disabled";
+	};
+
 	battery_power_supply: battery-power-supply {
 		compatible = "x-powers,axp803-battery-power-supply",
 			     "x-powers,axp813-battery-power-supply";
-- 
2.17.1

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

* [PATCH v2 4/8] arm64: dts: allwinner: axp803: add charge led node
@ 2019-02-15 11:50   ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

Add dt node for axp20x-led driver controlling CHGLED.
Default status is disabled, since it may be not used.

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 arch/arm64/boot/dts/allwinner/axp803.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/axp803.dtsi b/arch/arm64/boot/dts/allwinner/axp803.dtsi
index c3a618e1279a..af4898602b34 100644
--- a/arch/arm64/boot/dts/allwinner/axp803.dtsi
+++ b/arch/arm64/boot/dts/allwinner/axp803.dtsi
@@ -76,6 +76,11 @@
 		};
 	};
 
+	axp_led: led {
+		compatible = "x-powers,axp20x-led";
+		status = "disabled";
+	};
+
 	battery_power_supply: battery-power-supply {
 		compatible = "x-powers,axp803-battery-power-supply",
 			     "x-powers,axp813-battery-power-supply";
-- 
2.17.1


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

* [PATCH v2 4/8] arm64: dts: allwinner: axp803: add charge led node
@ 2019-02-15 11:50   ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

Add dt node for axp20x-led driver controlling CHGLED.
Default status is disabled, since it may be not used.

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 arch/arm64/boot/dts/allwinner/axp803.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/axp803.dtsi b/arch/arm64/boot/dts/allwinner/axp803.dtsi
index c3a618e1279a..af4898602b34 100644
--- a/arch/arm64/boot/dts/allwinner/axp803.dtsi
+++ b/arch/arm64/boot/dts/allwinner/axp803.dtsi
@@ -76,6 +76,11 @@
 		};
 	};
 
+	axp_led: led {
+		compatible = "x-powers,axp20x-led";
+		status = "disabled";
+	};
+
 	battery_power_supply: battery-power-supply {
 		compatible = "x-powers,axp803-battery-power-supply",
 			     "x-powers,axp813-battery-power-supply";
-- 
2.17.1


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

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

* [PATCH v2 5/8] arm64: dts: allwinner: Enable AXP803 CHGLED for Olimex boards
  2019-02-15 11:50 ` Stefan Mavrodiev
  (?)
@ 2019-02-15 11:50   ` Stefan Mavrodiev
  -1 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

Teres-I and A64-OLinuXino commes with populated LED connected
to AXP803. So to be used as battery indication, pass the LED control
to the charger itself in mode 0 ( FULL while charging, OFF no battery
or completed).

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts | 6 ++++++
 arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts   | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
index f7a4bccaa5d4..7d6930319fba 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
@@ -177,6 +177,12 @@
 
 #include "axp803.dtsi"
 
+&axp_led {
+	label = "a64-olinuxino:yellow:chgled";
+	status = "okay";
+	x-powers,charger-mode = <0>;
+};
+
 &reg_aldo1 {
 	regulator-always-on;
 	regulator-min-microvolt = <2800000>;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
index c455b24dd079..cdffdd4e4561 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
@@ -145,6 +145,12 @@
 
 #include "axp803.dtsi"
 
+&axp_led {
+	label = "teres-i:yellow:chgled";
+	status = "okay";
+	x-powers,charger-mode = <0>;
+};
+
 &reg_aldo1 {
 	regulator-always-on;
 	regulator-min-microvolt = <2800000>;
-- 
2.17.1

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

* [PATCH v2 5/8] arm64: dts: allwinner: Enable AXP803 CHGLED for Olimex boards
@ 2019-02-15 11:50   ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

Teres-I and A64-OLinuXino commes with populated LED connected
to AXP803. So to be used as battery indication, pass the LED control
to the charger itself in mode 0 ( FULL while charging, OFF no battery
or completed).

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts | 6 ++++++
 arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts   | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
index f7a4bccaa5d4..7d6930319fba 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
@@ -177,6 +177,12 @@
 
 #include "axp803.dtsi"
 
+&axp_led {
+	label = "a64-olinuxino:yellow:chgled";
+	status = "okay";
+	x-powers,charger-mode = <0>;
+};
+
 &reg_aldo1 {
 	regulator-always-on;
 	regulator-min-microvolt = <2800000>;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
index c455b24dd079..cdffdd4e4561 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
@@ -145,6 +145,12 @@
 
 #include "axp803.dtsi"
 
+&axp_led {
+	label = "teres-i:yellow:chgled";
+	status = "okay";
+	x-powers,charger-mode = <0>;
+};
+
 &reg_aldo1 {
 	regulator-always-on;
 	regulator-min-microvolt = <2800000>;
-- 
2.17.1


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

* [PATCH v2 5/8] arm64: dts: allwinner: Enable AXP803 CHGLED for Olimex boards
@ 2019-02-15 11:50   ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

Teres-I and A64-OLinuXino commes with populated LED connected
to AXP803. So to be used as battery indication, pass the LED control
to the charger itself in mode 0 ( FULL while charging, OFF no battery
or completed).

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts | 6 ++++++
 arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts   | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
index f7a4bccaa5d4..7d6930319fba 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
@@ -177,6 +177,12 @@
 
 #include "axp803.dtsi"
 
+&axp_led {
+	label = "a64-olinuxino:yellow:chgled";
+	status = "okay";
+	x-powers,charger-mode = <0>;
+};
+
 &reg_aldo1 {
 	regulator-always-on;
 	regulator-min-microvolt = <2800000>;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
index c455b24dd079..cdffdd4e4561 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
@@ -145,6 +145,12 @@
 
 #include "axp803.dtsi"
 
+&axp_led {
+	label = "teres-i:yellow:chgled";
+	status = "okay";
+	x-powers,charger-mode = <0>;
+};
+
 &reg_aldo1 {
 	regulator-always-on;
 	regulator-min-microvolt = <2800000>;
-- 
2.17.1


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

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

* [PATCH v2 6/8] arm: dts: axpxx: add charge led node
  2019-02-15 11:50 ` Stefan Mavrodiev
  (?)
@ 2019-02-15 11:50   ` Stefan Mavrodiev
  -1 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

Add dt node for axp20x-led driver controlling CHGLED.
Default status is disabled, since it may be not used.

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 arch/arm/boot/dts/axp209.dtsi | 5 +++++
 arch/arm/boot/dts/axp22x.dtsi | 5 +++++
 arch/arm/boot/dts/axp81x.dtsi | 5 +++++
 3 files changed, 15 insertions(+)

diff --git a/arch/arm/boot/dts/axp209.dtsi b/arch/arm/boot/dts/axp209.dtsi
index 0d9ff12bdf28..f972b6f3ecd0 100644
--- a/arch/arm/boot/dts/axp209.dtsi
+++ b/arch/arm/boot/dts/axp209.dtsi
@@ -69,6 +69,11 @@
 		#gpio-cells = <2>;
 	};
 
+	axp_led: led {
+		compatible = "x-powers,axp20x-led";
+		status = "disabled";
+	};
+
 	battery_power_supply: battery-power-supply {
 		compatible = "x-powers,axp209-battery-power-supply";
 		status = "disabled";
diff --git a/arch/arm/boot/dts/axp22x.dtsi b/arch/arm/boot/dts/axp22x.dtsi
index 65a07a67aca9..92a0b64252b1 100644
--- a/arch/arm/boot/dts/axp22x.dtsi
+++ b/arch/arm/boot/dts/axp22x.dtsi
@@ -62,6 +62,11 @@
 		#io-channel-cells = <1>;
 	};
 
+	axp_led: led {
+		compatible = "x-powers,axp20x-led";
+		status = "disabled";
+	};
+
 	battery_power_supply: battery-power-supply {
 		compatible = "x-powers,axp221-battery-power-supply";
 		status = "disabled";
diff --git a/arch/arm/boot/dts/axp81x.dtsi b/arch/arm/boot/dts/axp81x.dtsi
index bd83962d3627..22e243cc40d5 100644
--- a/arch/arm/boot/dts/axp81x.dtsi
+++ b/arch/arm/boot/dts/axp81x.dtsi
@@ -74,6 +74,11 @@
 		};
 	};
 
+	axp_led: led {
+		compatible = "x-powers,axp20x-led";
+		status = "disabled";
+	};
+
 	battery_power_supply: battery-power-supply {
 		compatible = "x-powers,axp813-battery-power-supply";
 		status = "disabled";
-- 
2.17.1

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

* [PATCH v2 6/8] arm: dts: axpxx: add charge led node
@ 2019-02-15 11:50   ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

Add dt node for axp20x-led driver controlling CHGLED.
Default status is disabled, since it may be not used.

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 arch/arm/boot/dts/axp209.dtsi | 5 +++++
 arch/arm/boot/dts/axp22x.dtsi | 5 +++++
 arch/arm/boot/dts/axp81x.dtsi | 5 +++++
 3 files changed, 15 insertions(+)

diff --git a/arch/arm/boot/dts/axp209.dtsi b/arch/arm/boot/dts/axp209.dtsi
index 0d9ff12bdf28..f972b6f3ecd0 100644
--- a/arch/arm/boot/dts/axp209.dtsi
+++ b/arch/arm/boot/dts/axp209.dtsi
@@ -69,6 +69,11 @@
 		#gpio-cells = <2>;
 	};
 
+	axp_led: led {
+		compatible = "x-powers,axp20x-led";
+		status = "disabled";
+	};
+
 	battery_power_supply: battery-power-supply {
 		compatible = "x-powers,axp209-battery-power-supply";
 		status = "disabled";
diff --git a/arch/arm/boot/dts/axp22x.dtsi b/arch/arm/boot/dts/axp22x.dtsi
index 65a07a67aca9..92a0b64252b1 100644
--- a/arch/arm/boot/dts/axp22x.dtsi
+++ b/arch/arm/boot/dts/axp22x.dtsi
@@ -62,6 +62,11 @@
 		#io-channel-cells = <1>;
 	};
 
+	axp_led: led {
+		compatible = "x-powers,axp20x-led";
+		status = "disabled";
+	};
+
 	battery_power_supply: battery-power-supply {
 		compatible = "x-powers,axp221-battery-power-supply";
 		status = "disabled";
diff --git a/arch/arm/boot/dts/axp81x.dtsi b/arch/arm/boot/dts/axp81x.dtsi
index bd83962d3627..22e243cc40d5 100644
--- a/arch/arm/boot/dts/axp81x.dtsi
+++ b/arch/arm/boot/dts/axp81x.dtsi
@@ -74,6 +74,11 @@
 		};
 	};
 
+	axp_led: led {
+		compatible = "x-powers,axp20x-led";
+		status = "disabled";
+	};
+
 	battery_power_supply: battery-power-supply {
 		compatible = "x-powers,axp813-battery-power-supply";
 		status = "disabled";
-- 
2.17.1


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

* [PATCH v2 6/8] arm: dts: axpxx: add charge led node
@ 2019-02-15 11:50   ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

Add dt node for axp20x-led driver controlling CHGLED.
Default status is disabled, since it may be not used.

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 arch/arm/boot/dts/axp209.dtsi | 5 +++++
 arch/arm/boot/dts/axp22x.dtsi | 5 +++++
 arch/arm/boot/dts/axp81x.dtsi | 5 +++++
 3 files changed, 15 insertions(+)

diff --git a/arch/arm/boot/dts/axp209.dtsi b/arch/arm/boot/dts/axp209.dtsi
index 0d9ff12bdf28..f972b6f3ecd0 100644
--- a/arch/arm/boot/dts/axp209.dtsi
+++ b/arch/arm/boot/dts/axp209.dtsi
@@ -69,6 +69,11 @@
 		#gpio-cells = <2>;
 	};
 
+	axp_led: led {
+		compatible = "x-powers,axp20x-led";
+		status = "disabled";
+	};
+
 	battery_power_supply: battery-power-supply {
 		compatible = "x-powers,axp209-battery-power-supply";
 		status = "disabled";
diff --git a/arch/arm/boot/dts/axp22x.dtsi b/arch/arm/boot/dts/axp22x.dtsi
index 65a07a67aca9..92a0b64252b1 100644
--- a/arch/arm/boot/dts/axp22x.dtsi
+++ b/arch/arm/boot/dts/axp22x.dtsi
@@ -62,6 +62,11 @@
 		#io-channel-cells = <1>;
 	};
 
+	axp_led: led {
+		compatible = "x-powers,axp20x-led";
+		status = "disabled";
+	};
+
 	battery_power_supply: battery-power-supply {
 		compatible = "x-powers,axp221-battery-power-supply";
 		status = "disabled";
diff --git a/arch/arm/boot/dts/axp81x.dtsi b/arch/arm/boot/dts/axp81x.dtsi
index bd83962d3627..22e243cc40d5 100644
--- a/arch/arm/boot/dts/axp81x.dtsi
+++ b/arch/arm/boot/dts/axp81x.dtsi
@@ -74,6 +74,11 @@
 		};
 	};
 
+	axp_led: led {
+		compatible = "x-powers,axp20x-led";
+		status = "disabled";
+	};
+
 	battery_power_supply: battery-power-supply {
 		compatible = "x-powers,axp813-battery-power-supply";
 		status = "disabled";
-- 
2.17.1


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

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

* [PATCH v2 7/8] ARM: dts: sun7i: Enable AXP209 CHGLED for Olimex boards
  2019-02-15 11:50 ` Stefan Mavrodiev
  (?)
@ 2019-02-15 11:50   ` Stefan Mavrodiev
  -1 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

Olimex A20-OLinuXino based boards (MICRO, SOM, SOM204, LIME2)
commes with populated LED connected to AXP209.

By default the LED is controlled by AXP209, so this binding
actually doesn't modify any registers. However this can can be
used as general purpose LED, if the control mode is overridden.

Also this binding is enabled only for OLIMEX boards, since I have
no knowlegde if the other manifactures are populating this LED.

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts    | 6 ++++++
 arch/arm/boot/dts/sun7i-a20-olimex-som204-evb.dts | 6 ++++++
 arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts   | 6 ++++++
 arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts   | 6 ++++++
 4 files changed, 24 insertions(+)

diff --git a/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts b/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts
index f0e6a96e5785..677ee1c2795a 100644
--- a/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts
+++ b/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts
@@ -243,6 +243,12 @@
 
 #include "axp209.dtsi"
 
+&axp_led {
+	label = "a20-olimex-som-evb:yellow:chgled";
+	status = "okay";
+	x-powers,charger-mode = <0>;
+};
+
 &reg_dcdc2 {
 	regulator-always-on;
 	regulator-min-microvolt = <1000000>;
diff --git a/arch/arm/boot/dts/sun7i-a20-olimex-som204-evb.dts b/arch/arm/boot/dts/sun7i-a20-olimex-som204-evb.dts
index 823aabce0462..31a3ab5ad4e3 100644
--- a/arch/arm/boot/dts/sun7i-a20-olimex-som204-evb.dts
+++ b/arch/arm/boot/dts/sun7i-a20-olimex-som204-evb.dts
@@ -205,6 +205,12 @@
 	status = "okay";
 };
 
+&axp_led {
+	label = "a20-som204-evb:yellow:chgled";
+	status = "okay";
+	x-powers,charger-mode = <0>;
+};
+
 &battery_power_supply {
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
index 4e1c590eb098..66dd80ced1fa 100644
--- a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
+++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
@@ -200,6 +200,12 @@
 
 #include "axp209.dtsi"
 
+&axp_led {
+	label = "a20-olinuxino-lime2:yellow:chgled";
+	status = "okay";
+	x-powers,charger-mode = <0>;
+};
+
 &reg_dcdc2 {
 	regulator-always-on;
 	regulator-min-microvolt = <1000000>;
diff --git a/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts b/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
index 840ae1194a66..700de909eb49 100644
--- a/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
+++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
@@ -272,6 +272,12 @@
 	status = "okay";
 };
 
+&axp_led {
+	label = "a20-olinuxino-micro:yellow:chgled";
+	status = "okay";
+	x-powers,charger-mode = <0>;
+};
+
 &battery_power_supply {
 	status = "okay";
 };
-- 
2.17.1

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

* [PATCH v2 7/8] ARM: dts: sun7i: Enable AXP209 CHGLED for Olimex boards
@ 2019-02-15 11:50   ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

Olimex A20-OLinuXino based boards (MICRO, SOM, SOM204, LIME2)
commes with populated LED connected to AXP209.

By default the LED is controlled by AXP209, so this binding
actually doesn't modify any registers. However this can can be
used as general purpose LED, if the control mode is overridden.

Also this binding is enabled only for OLIMEX boards, since I have
no knowlegde if the other manifactures are populating this LED.

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts    | 6 ++++++
 arch/arm/boot/dts/sun7i-a20-olimex-som204-evb.dts | 6 ++++++
 arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts   | 6 ++++++
 arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts   | 6 ++++++
 4 files changed, 24 insertions(+)

diff --git a/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts b/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts
index f0e6a96e5785..677ee1c2795a 100644
--- a/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts
+++ b/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts
@@ -243,6 +243,12 @@
 
 #include "axp209.dtsi"
 
+&axp_led {
+	label = "a20-olimex-som-evb:yellow:chgled";
+	status = "okay";
+	x-powers,charger-mode = <0>;
+};
+
 &reg_dcdc2 {
 	regulator-always-on;
 	regulator-min-microvolt = <1000000>;
diff --git a/arch/arm/boot/dts/sun7i-a20-olimex-som204-evb.dts b/arch/arm/boot/dts/sun7i-a20-olimex-som204-evb.dts
index 823aabce0462..31a3ab5ad4e3 100644
--- a/arch/arm/boot/dts/sun7i-a20-olimex-som204-evb.dts
+++ b/arch/arm/boot/dts/sun7i-a20-olimex-som204-evb.dts
@@ -205,6 +205,12 @@
 	status = "okay";
 };
 
+&axp_led {
+	label = "a20-som204-evb:yellow:chgled";
+	status = "okay";
+	x-powers,charger-mode = <0>;
+};
+
 &battery_power_supply {
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
index 4e1c590eb098..66dd80ced1fa 100644
--- a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
+++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
@@ -200,6 +200,12 @@
 
 #include "axp209.dtsi"
 
+&axp_led {
+	label = "a20-olinuxino-lime2:yellow:chgled";
+	status = "okay";
+	x-powers,charger-mode = <0>;
+};
+
 &reg_dcdc2 {
 	regulator-always-on;
 	regulator-min-microvolt = <1000000>;
diff --git a/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts b/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
index 840ae1194a66..700de909eb49 100644
--- a/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
+++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
@@ -272,6 +272,12 @@
 	status = "okay";
 };
 
+&axp_led {
+	label = "a20-olinuxino-micro:yellow:chgled";
+	status = "okay";
+	x-powers,charger-mode = <0>;
+};
+
 &battery_power_supply {
 	status = "okay";
 };
-- 
2.17.1


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

* [PATCH v2 7/8] ARM: dts: sun7i: Enable AXP209 CHGLED for Olimex boards
@ 2019-02-15 11:50   ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

Olimex A20-OLinuXino based boards (MICRO, SOM, SOM204, LIME2)
commes with populated LED connected to AXP209.

By default the LED is controlled by AXP209, so this binding
actually doesn't modify any registers. However this can can be
used as general purpose LED, if the control mode is overridden.

Also this binding is enabled only for OLIMEX boards, since I have
no knowlegde if the other manifactures are populating this LED.

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts    | 6 ++++++
 arch/arm/boot/dts/sun7i-a20-olimex-som204-evb.dts | 6 ++++++
 arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts   | 6 ++++++
 arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts   | 6 ++++++
 4 files changed, 24 insertions(+)

diff --git a/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts b/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts
index f0e6a96e5785..677ee1c2795a 100644
--- a/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts
+++ b/arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts
@@ -243,6 +243,12 @@
 
 #include "axp209.dtsi"
 
+&axp_led {
+	label = "a20-olimex-som-evb:yellow:chgled";
+	status = "okay";
+	x-powers,charger-mode = <0>;
+};
+
 &reg_dcdc2 {
 	regulator-always-on;
 	regulator-min-microvolt = <1000000>;
diff --git a/arch/arm/boot/dts/sun7i-a20-olimex-som204-evb.dts b/arch/arm/boot/dts/sun7i-a20-olimex-som204-evb.dts
index 823aabce0462..31a3ab5ad4e3 100644
--- a/arch/arm/boot/dts/sun7i-a20-olimex-som204-evb.dts
+++ b/arch/arm/boot/dts/sun7i-a20-olimex-som204-evb.dts
@@ -205,6 +205,12 @@
 	status = "okay";
 };
 
+&axp_led {
+	label = "a20-som204-evb:yellow:chgled";
+	status = "okay";
+	x-powers,charger-mode = <0>;
+};
+
 &battery_power_supply {
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
index 4e1c590eb098..66dd80ced1fa 100644
--- a/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
+++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
@@ -200,6 +200,12 @@
 
 #include "axp209.dtsi"
 
+&axp_led {
+	label = "a20-olinuxino-lime2:yellow:chgled";
+	status = "okay";
+	x-powers,charger-mode = <0>;
+};
+
 &reg_dcdc2 {
 	regulator-always-on;
 	regulator-min-microvolt = <1000000>;
diff --git a/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts b/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
index 840ae1194a66..700de909eb49 100644
--- a/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
+++ b/arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
@@ -272,6 +272,12 @@
 	status = "okay";
 };
 
+&axp_led {
+	label = "a20-olinuxino-micro:yellow:chgled";
+	status = "okay";
+	x-powers,charger-mode = <0>;
+};
+
 &battery_power_supply {
 	status = "okay";
 };
-- 
2.17.1


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

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

* [PATCH v2 8/8] ARM: dts: sun8i: a33: Enable AXP223 CHGLED for A33-OLinuXino
  2019-02-15 11:50 ` Stefan Mavrodiev
  (?)
@ 2019-02-15 11:50   ` Stefan Mavrodiev
  -1 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

A33-OLinuXino comes with populated CHGLED LED indicating battery
charging status. By default the control is set to manual mode,
which doesn't indicate battery status. So it makes sense to set
the control to automatic mode 0 ( FULL - charging, OFF - no battery
or battery full).

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 arch/arm/boot/dts/sun8i-a33-olinuxino.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a33-olinuxino.dts b/arch/arm/boot/dts/sun8i-a33-olinuxino.dts
index 3d78169cdeed..a1e36ee51bbb 100644
--- a/arch/arm/boot/dts/sun8i-a33-olinuxino.dts
+++ b/arch/arm/boot/dts/sun8i-a33-olinuxino.dts
@@ -111,6 +111,12 @@
 	status = "okay";
 };
 
+&axp_led {
+	label = "a33-olinuxino:yellow:chgled";
+	status = "okay";
+	x-powers,charger-mode = <0>;
+};
+
 &battery_power_supply {
 	status = "okay";
 };
-- 
2.17.1

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

* [PATCH v2 8/8] ARM: dts: sun8i: a33: Enable AXP223 CHGLED for A33-OLinuXino
@ 2019-02-15 11:50   ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

A33-OLinuXino comes with populated CHGLED LED indicating battery
charging status. By default the control is set to manual mode,
which doesn't indicate battery status. So it makes sense to set
the control to automatic mode 0 ( FULL - charging, OFF - no battery
or battery full).

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 arch/arm/boot/dts/sun8i-a33-olinuxino.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a33-olinuxino.dts b/arch/arm/boot/dts/sun8i-a33-olinuxino.dts
index 3d78169cdeed..a1e36ee51bbb 100644
--- a/arch/arm/boot/dts/sun8i-a33-olinuxino.dts
+++ b/arch/arm/boot/dts/sun8i-a33-olinuxino.dts
@@ -111,6 +111,12 @@
 	status = "okay";
 };
 
+&axp_led {
+	label = "a33-olinuxino:yellow:chgled";
+	status = "okay";
+	x-powers,charger-mode = <0>;
+};
+
 &battery_power_supply {
 	status = "okay";
 };
-- 
2.17.1


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

* [PATCH v2 8/8] ARM: dts: sun8i: a33: Enable AXP223 CHGLED for A33-OLinuXino
@ 2019-02-15 11:50   ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 11:50 UTC (permalink / raw)
  To: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support
  Cc: Stefan Mavrodiev

A33-OLinuXino comes with populated CHGLED LED indicating battery
charging status. By default the control is set to manual mode,
which doesn't indicate battery status. So it makes sense to set
the control to automatic mode 0 ( FULL - charging, OFF - no battery
or battery full).

Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
---
 arch/arm/boot/dts/sun8i-a33-olinuxino.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a33-olinuxino.dts b/arch/arm/boot/dts/sun8i-a33-olinuxino.dts
index 3d78169cdeed..a1e36ee51bbb 100644
--- a/arch/arm/boot/dts/sun8i-a33-olinuxino.dts
+++ b/arch/arm/boot/dts/sun8i-a33-olinuxino.dts
@@ -111,6 +111,12 @@
 	status = "okay";
 };
 
+&axp_led {
+	label = "a33-olinuxino:yellow:chgled";
+	status = "okay";
+	x-powers,charger-mode = <0>;
+};
+
 &battery_power_supply {
 	status = "okay";
 };
-- 
2.17.1


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

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

* Re: [PATCH v2 1/8] leds: Add support for AXP20X CHGLED
  2019-02-15 11:50   ` Stefan Mavrodiev
  (?)
@ 2019-02-15 14:07     ` Stefan Mavrodiev
  -1 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 14:07 UTC (permalink / raw)
  To: Stefan Mavrodiev, Jacek Anaszewski, Pavel Machek, Rob Herring,
	Mark Rutland, Chen-Yu Tsai, Maxime Ripard, Lee Jones,
	open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support


On 2/15/19 1:50 PM, Stefan Mavrodiev wrote:
> Most of AXP20x PMIC chips have built-in battery charger with LED indicator.
> The LED can be controlled ether by the charger or manually by a register.
>
> The default is (except for AXP209) manual control, which makes this LED
> useless, since there is no device driver.
>
> The driver rely on AXP20X MFD driver.
>
> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
> ---
>   drivers/leds/Kconfig       |  10 ++
>   drivers/leds/Makefile      |   1 +
>   drivers/leds/leds-axp20x.c | 291 +++++++++++++++++++++++++++++++++++++
>   3 files changed, 302 insertions(+)
>   create mode 100644 drivers/leds/leds-axp20x.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index a72f97fca57b..82dce9063d41 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -766,6 +766,16 @@ config LEDS_NIC78BX
>   	  To compile this driver as a module, choose M here: the module
>   	  will be called leds-nic78bx.
>   
> +config LEDS_AXP20X
> +	tristate "LED support for X-Powers PMICs"
> +	depends on MFD_AXP20X
> +	help
> +	  This option enables support for CHGLED found on most of X-Powers
> +	  PMICs.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called leds-axp20x.
> +
>   comment "LED Triggers"
>   source "drivers/leds/trigger/Kconfig"
>   
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 4c1b0054f379..d3fb76e119d8 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -79,6 +79,7 @@ obj-$(CONFIG_LEDS_MT6323)		+= leds-mt6323.o
>   obj-$(CONFIG_LEDS_LM3692X)		+= leds-lm3692x.o
>   obj-$(CONFIG_LEDS_SC27XX_BLTC)		+= leds-sc27xx-bltc.o
>   obj-$(CONFIG_LEDS_LM3601X)		+= leds-lm3601x.o
> +obj-$(CONFIG_LEDS_AXP20X)		+= leds-axp20x.o
>   
>   # LED SPI Drivers
>   obj-$(CONFIG_LEDS_CR0014114)		+= leds-cr0014114.o
> diff --git a/drivers/leds/leds-axp20x.c b/drivers/leds/leds-axp20x.c
> new file mode 100644
> index 000000000000..2d5ae1c085f0
> --- /dev/null
> +++ b/drivers/leds/leds-axp20x.c
> @@ -0,0 +1,291 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +//
> +// Copyright 2019 Stefan Mavrodiev <stefan@olimex.com>
> +

The copyright header is wrong. It should be "Copyright 2019 Olimex Ltd.".

I'd like to fix it before the patchis merged (eventually).

Regards,
Stefan Mavrodiev

> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/leds.h>
> +#include <linux/mfd/axp20x.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <uapi/linux/uleds.h>
> +
> +
> +#define AXP20X_CHGLED_CTRL_REG		AXP20X_OFF_CTRL
> +#define AXP20X_CHGLED_FUNC_MASK			GENMASK(5, 4)
> +#define AXP20X_CHGLED_FUNC_OFF			(0 << 4)
> +#define AXP20X_CHGLED_FUNC_1HZ			(1 << 4)
> +#define AXP20X_CHGLED_FUNC_4HZ			(2 << 4)
> +#define AXP20X_CHGLED_FUNC_FULL			(3 << 4)
> +#define AXP20X_CHGLED_CTRL_MASK			BIT(3)
> +#define AXP20X_CHGLED_CTRL_MANUAL		0
> +#define AXP20X_CHGLED_CTRL_CHARGER		1
> +#define AXP20X_CHGLED_CTRL(_ctrl)		(_ctrl << 3)
> +
> +#define AXP20X_CHGLED_MODE_REG		AXP20X_CHRG_CTRL2
> +#define AXP20X_CHGLED_MODE_MASK			BIT(4)
> +#define AXP20X_CHGLED_MODE_A			0
> +#define AXP20X_CHGLED_MODE_B			1
> +#define AXP20X_CHGLED_MODE(_mode)		(_mode << 4)
> +
> +struct axp20x_led {
> +	char			name[LED_MAX_NAME_SIZE];
> +	struct led_classdev	cdev;
> +	struct mutex		lock;
> +	u8			mode : 1;
> +	u8			ctrl : 1;
> +	u8			ctrl_inverted : 1;
> +	struct axp20x_dev	*axp20x;
> +};
> +
> +static inline struct axp20x_led *to_axp20x_led(struct led_classdev *cdev)
> +{
> +	return container_of(cdev, struct axp20x_led, cdev);
> +}
> +
> +static int axp20x_led_setup(struct axp20x_led *priv)
> +{
> +	int ret;
> +	u8 val;
> +
> +	/* Invert the logic, if necessary */
> +	val = priv->ctrl ^ priv->ctrl_inverted;
> +
> +	mutex_lock(&priv->lock);
> +	ret = regmap_update_bits(priv->axp20x->regmap, AXP20X_CHGLED_CTRL_REG,
> +				 AXP20X_CHGLED_CTRL_MASK,
> +				 AXP20X_CHGLED_CTRL(val));
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = regmap_update_bits(priv->axp20x->regmap, AXP20X_CHGLED_MODE_REG,
> +				 AXP20X_CHGLED_MODE_MASK,
> +				 AXP20X_CHGLED_MODE(priv->mode));
> +out:
> +	mutex_unlock(&priv->lock);
> +	return ret;
> +}
> +
> +static ssize_t control_show(struct device *dev, struct device_attribute *attr,
> +			    char *buf)
> +{
> +	struct led_classdev *cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +
> +	return sprintf(buf, "%u\n", priv->ctrl);
> +}
> +
> +static ssize_t control_store(struct device *dev, struct device_attribute *attr,
> +			     const char *buf, size_t size)
> +{
> +	struct led_classdev *cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +	unsigned long val;
> +	int ret;
> +
> +	ret = kstrtoul(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	/**
> +	 * Supported values are:
> +	 *   - 0 : Manual control
> +	 *   - 1 : Charger control
> +	 */
> +	if (val > 1)
> +		return -EINVAL;
> +
> +	priv->ctrl = val;
> +
> +	return axp20x_led_setup(priv) ? : size;
> +}
> +static DEVICE_ATTR_RW(control);
> +
> +static ssize_t mode_show(struct device *dev, struct device_attribute *attr,
> +			 char *buf)
> +{
> +	struct led_classdev *cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +
> +	return sprintf(buf, "%u\n", priv->mode);
> +}
> +
> +static ssize_t mode_store(struct device *dev, struct device_attribute *attr,
> +			  const char *buf, size_t size)
> +{
> +	struct led_classdev *cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +	unsigned long val;
> +	int ret;
> +
> +	ret = kstrtoul(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +	/**
> +	 * Supported values are:
> +	 *   - 0 : Mode A
> +	 *   - 1 : Mode B
> +	 */
> +	if (val > 1)
> +		return -EINVAL;
> +
> +	priv->mode = val;
> +
> +	return axp20x_led_setup(priv) ? : size;
> +}
> +static DEVICE_ATTR_RW(mode);
> +
> +static struct attribute *axp20x_led_attrs[] = {
> +	&dev_attr_control.attr,
> +	&dev_attr_mode.attr,
> +	NULL,
> +};
> +ATTRIBUTE_GROUPS(axp20x_led);
> +
> +enum led_brightness axp20x_led_brightness_get(struct led_classdev *cdev)
> +{
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +	u32 val;
> +	int ret;
> +
> +	mutex_lock(&priv->lock);
> +	ret = regmap_read(priv->axp20x->regmap, AXP20X_CHGLED_CTRL_REG, &val);
> +	mutex_unlock(&priv->lock);
> +	if (ret < 0)
> +		return LED_OFF;
> +
> +	return (val & AXP20X_CHGLED_FUNC_FULL) ? LED_FULL : LED_OFF;
> +}
> +
> +static int axp20x_led_brightness_set_blocking(struct led_classdev *cdev,
> +					      enum led_brightness brightness)
> +{
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +	int ret = 0;
> +
> +	mutex_lock(&priv->lock);
> +	ret = regmap_update_bits(priv->axp20x->regmap,
> +				 AXP20X_CHGLED_CTRL_REG,
> +				 AXP20X_CHGLED_FUNC_MASK,
> +				 (brightness) ?
> +				 AXP20X_CHGLED_FUNC_FULL :
> +				 AXP20X_CHGLED_FUNC_OFF);
> +	mutex_unlock(&priv->lock);
> +
> +	return ret;
> +}
> +
> +static int axp20x_led_parse_dt(struct axp20x_led *priv, struct device_node *np)
> +{
> +	const char *str;
> +	u8 value;
> +	int ret = 0;
> +
> +	str = of_get_property(np, "label", NULL);
> +	if (!str)
> +		snprintf(priv->name, sizeof(priv->name), "axp20x::");
> +	else
> +		snprintf(priv->name, sizeof(priv->name), "axp20x:%s", str);
> +	priv->cdev.name = priv->name;
> +
> +	priv->cdev.default_trigger = of_get_property(np,
> +						     "linux,default-trigger",
> +						     NULL);
> +
> +	if (!of_property_read_u8(np, "x-powers,charger-mode", &value)) {
> +		priv->ctrl = AXP20X_CHGLED_CTRL_CHARGER;
> +		priv->mode = (value < 2) ? value : 0;
> +	} else {
> +		priv->ctrl = AXP20X_CHGLED_CTRL_MANUAL;
> +	}
> +
> +	str = of_get_property(np, "default-state", NULL);
> +	if (str) {
> +		if (!strcmp(str, "keep")) {
> +			ret = axp20x_led_brightness_get(&priv->cdev);
> +			if (ret < 0)
> +				return ret;
> +			priv->cdev.brightness = ret;
> +		} else if (!strcmp(str, "on")) {
> +			ret = axp20x_led_brightness_set_blocking(&priv->cdev,
> +								 LED_FULL);
> +		} else  {
> +			ret = axp20x_led_brightness_set_blocking(&priv->cdev,
> +								 LED_OFF);
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct of_device_id axp20x_led_of_match[] = {
> +	{ .compatible = "x-powers,axp20x-led" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, axp20x_led_of_match);
> +
> +static int axp20x_led_probe(struct platform_device *pdev)
> +{
> +	struct axp20x_led *priv;
> +	int ret;
> +
> +	if (!of_device_is_available(pdev->dev.of_node))
> +		return -ENODEV;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(struct axp20x_led),
> +			    GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->axp20x = dev_get_drvdata(pdev->dev.parent);
> +	if (!priv->axp20x) {
> +		dev_err(&pdev->dev, "Failed to get parent data\n");
> +		return -ENXIO;
> +	}
> +
> +	mutex_init(&priv->lock);
> +
> +	priv->cdev.brightness_set_blocking = axp20x_led_brightness_set_blocking;
> +	priv->cdev.brightness_get = axp20x_led_brightness_get;
> +	priv->cdev.groups = axp20x_led_groups;
> +
> +	ret = axp20x_led_parse_dt(priv, pdev->dev.of_node);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Failed to set parameters\n");
> +		return ret;
> +	}
> +
> +	/**
> +	 * For some reason in AXP209 the bit that controls CHGLED is with
> +	 * inverted logic compared to all other PMICs.
> +	 * If the PMIC is actually AXP209, set inverted flag and later use it
> +	 * when configuring the LED.
> +	 */
> +	if (priv->axp20x->variant == AXP209_ID)
> +		priv->ctrl_inverted = 1;
> +
> +	ret =  axp20x_led_setup(priv);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Failed to configure led");
> +		return ret;
> +	}
> +
> +	return devm_led_classdev_register(&pdev->dev, &priv->cdev);
> +}
> +
> +static struct platform_driver axp20x_led_driver = {
> +	.driver = {
> +		.name	= "axp20x-led",
> +		.of_match_table = of_match_ptr(axp20x_led_of_match),
> +	},
> +	.probe = axp20x_led_probe,
> +};
> +
> +module_platform_driver(axp20x_led_driver);
> +
> +MODULE_AUTHOR("Stefan Mavrodiev <stefan@olimex.com");
> +MODULE_DESCRIPTION("X-Powers PMIC CHGLED driver");
> +MODULE_LICENSE("GPL");

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

* Re: [PATCH v2 1/8] leds: Add support for AXP20X CHGLED
@ 2019-02-15 14:07     ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 14:07 UTC (permalink / raw)
  To: Stefan Mavrodiev, Jacek Anaszewski, Pavel Machek, Rob Herring,
	Mark Rutland, Chen-Yu Tsai, Maxime Ripard, Lee Jones,
	open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support


On 2/15/19 1:50 PM, Stefan Mavrodiev wrote:
> Most of AXP20x PMIC chips have built-in battery charger with LED indicator.
> The LED can be controlled ether by the charger or manually by a register.
>
> The default is (except for AXP209) manual control, which makes this LED
> useless, since there is no device driver.
>
> The driver rely on AXP20X MFD driver.
>
> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
> ---
>   drivers/leds/Kconfig       |  10 ++
>   drivers/leds/Makefile      |   1 +
>   drivers/leds/leds-axp20x.c | 291 +++++++++++++++++++++++++++++++++++++
>   3 files changed, 302 insertions(+)
>   create mode 100644 drivers/leds/leds-axp20x.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index a72f97fca57b..82dce9063d41 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -766,6 +766,16 @@ config LEDS_NIC78BX
>   	  To compile this driver as a module, choose M here: the module
>   	  will be called leds-nic78bx.
>   
> +config LEDS_AXP20X
> +	tristate "LED support for X-Powers PMICs"
> +	depends on MFD_AXP20X
> +	help
> +	  This option enables support for CHGLED found on most of X-Powers
> +	  PMICs.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called leds-axp20x.
> +
>   comment "LED Triggers"
>   source "drivers/leds/trigger/Kconfig"
>   
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 4c1b0054f379..d3fb76e119d8 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -79,6 +79,7 @@ obj-$(CONFIG_LEDS_MT6323)		+= leds-mt6323.o
>   obj-$(CONFIG_LEDS_LM3692X)		+= leds-lm3692x.o
>   obj-$(CONFIG_LEDS_SC27XX_BLTC)		+= leds-sc27xx-bltc.o
>   obj-$(CONFIG_LEDS_LM3601X)		+= leds-lm3601x.o
> +obj-$(CONFIG_LEDS_AXP20X)		+= leds-axp20x.o
>   
>   # LED SPI Drivers
>   obj-$(CONFIG_LEDS_CR0014114)		+= leds-cr0014114.o
> diff --git a/drivers/leds/leds-axp20x.c b/drivers/leds/leds-axp20x.c
> new file mode 100644
> index 000000000000..2d5ae1c085f0
> --- /dev/null
> +++ b/drivers/leds/leds-axp20x.c
> @@ -0,0 +1,291 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +//
> +// Copyright 2019 Stefan Mavrodiev <stefan@olimex.com>
> +

The copyright header is wrong. It should be "Copyright 2019 Olimex Ltd.".

I'd like to fix it before the patchis merged (eventually).

Regards,
Stefan Mavrodiev

> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/leds.h>
> +#include <linux/mfd/axp20x.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <uapi/linux/uleds.h>
> +
> +
> +#define AXP20X_CHGLED_CTRL_REG		AXP20X_OFF_CTRL
> +#define AXP20X_CHGLED_FUNC_MASK			GENMASK(5, 4)
> +#define AXP20X_CHGLED_FUNC_OFF			(0 << 4)
> +#define AXP20X_CHGLED_FUNC_1HZ			(1 << 4)
> +#define AXP20X_CHGLED_FUNC_4HZ			(2 << 4)
> +#define AXP20X_CHGLED_FUNC_FULL			(3 << 4)
> +#define AXP20X_CHGLED_CTRL_MASK			BIT(3)
> +#define AXP20X_CHGLED_CTRL_MANUAL		0
> +#define AXP20X_CHGLED_CTRL_CHARGER		1
> +#define AXP20X_CHGLED_CTRL(_ctrl)		(_ctrl << 3)
> +
> +#define AXP20X_CHGLED_MODE_REG		AXP20X_CHRG_CTRL2
> +#define AXP20X_CHGLED_MODE_MASK			BIT(4)
> +#define AXP20X_CHGLED_MODE_A			0
> +#define AXP20X_CHGLED_MODE_B			1
> +#define AXP20X_CHGLED_MODE(_mode)		(_mode << 4)
> +
> +struct axp20x_led {
> +	char			name[LED_MAX_NAME_SIZE];
> +	struct led_classdev	cdev;
> +	struct mutex		lock;
> +	u8			mode : 1;
> +	u8			ctrl : 1;
> +	u8			ctrl_inverted : 1;
> +	struct axp20x_dev	*axp20x;
> +};
> +
> +static inline struct axp20x_led *to_axp20x_led(struct led_classdev *cdev)
> +{
> +	return container_of(cdev, struct axp20x_led, cdev);
> +}
> +
> +static int axp20x_led_setup(struct axp20x_led *priv)
> +{
> +	int ret;
> +	u8 val;
> +
> +	/* Invert the logic, if necessary */
> +	val = priv->ctrl ^ priv->ctrl_inverted;
> +
> +	mutex_lock(&priv->lock);
> +	ret = regmap_update_bits(priv->axp20x->regmap, AXP20X_CHGLED_CTRL_REG,
> +				 AXP20X_CHGLED_CTRL_MASK,
> +				 AXP20X_CHGLED_CTRL(val));
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = regmap_update_bits(priv->axp20x->regmap, AXP20X_CHGLED_MODE_REG,
> +				 AXP20X_CHGLED_MODE_MASK,
> +				 AXP20X_CHGLED_MODE(priv->mode));
> +out:
> +	mutex_unlock(&priv->lock);
> +	return ret;
> +}
> +
> +static ssize_t control_show(struct device *dev, struct device_attribute *attr,
> +			    char *buf)
> +{
> +	struct led_classdev *cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +
> +	return sprintf(buf, "%u\n", priv->ctrl);
> +}
> +
> +static ssize_t control_store(struct device *dev, struct device_attribute *attr,
> +			     const char *buf, size_t size)
> +{
> +	struct led_classdev *cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +	unsigned long val;
> +	int ret;
> +
> +	ret = kstrtoul(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	/**
> +	 * Supported values are:
> +	 *   - 0 : Manual control
> +	 *   - 1 : Charger control
> +	 */
> +	if (val > 1)
> +		return -EINVAL;
> +
> +	priv->ctrl = val;
> +
> +	return axp20x_led_setup(priv) ? : size;
> +}
> +static DEVICE_ATTR_RW(control);
> +
> +static ssize_t mode_show(struct device *dev, struct device_attribute *attr,
> +			 char *buf)
> +{
> +	struct led_classdev *cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +
> +	return sprintf(buf, "%u\n", priv->mode);
> +}
> +
> +static ssize_t mode_store(struct device *dev, struct device_attribute *attr,
> +			  const char *buf, size_t size)
> +{
> +	struct led_classdev *cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +	unsigned long val;
> +	int ret;
> +
> +	ret = kstrtoul(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +	/**
> +	 * Supported values are:
> +	 *   - 0 : Mode A
> +	 *   - 1 : Mode B
> +	 */
> +	if (val > 1)
> +		return -EINVAL;
> +
> +	priv->mode = val;
> +
> +	return axp20x_led_setup(priv) ? : size;
> +}
> +static DEVICE_ATTR_RW(mode);
> +
> +static struct attribute *axp20x_led_attrs[] = {
> +	&dev_attr_control.attr,
> +	&dev_attr_mode.attr,
> +	NULL,
> +};
> +ATTRIBUTE_GROUPS(axp20x_led);
> +
> +enum led_brightness axp20x_led_brightness_get(struct led_classdev *cdev)
> +{
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +	u32 val;
> +	int ret;
> +
> +	mutex_lock(&priv->lock);
> +	ret = regmap_read(priv->axp20x->regmap, AXP20X_CHGLED_CTRL_REG, &val);
> +	mutex_unlock(&priv->lock);
> +	if (ret < 0)
> +		return LED_OFF;
> +
> +	return (val & AXP20X_CHGLED_FUNC_FULL) ? LED_FULL : LED_OFF;
> +}
> +
> +static int axp20x_led_brightness_set_blocking(struct led_classdev *cdev,
> +					      enum led_brightness brightness)
> +{
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +	int ret = 0;
> +
> +	mutex_lock(&priv->lock);
> +	ret = regmap_update_bits(priv->axp20x->regmap,
> +				 AXP20X_CHGLED_CTRL_REG,
> +				 AXP20X_CHGLED_FUNC_MASK,
> +				 (brightness) ?
> +				 AXP20X_CHGLED_FUNC_FULL :
> +				 AXP20X_CHGLED_FUNC_OFF);
> +	mutex_unlock(&priv->lock);
> +
> +	return ret;
> +}
> +
> +static int axp20x_led_parse_dt(struct axp20x_led *priv, struct device_node *np)
> +{
> +	const char *str;
> +	u8 value;
> +	int ret = 0;
> +
> +	str = of_get_property(np, "label", NULL);
> +	if (!str)
> +		snprintf(priv->name, sizeof(priv->name), "axp20x::");
> +	else
> +		snprintf(priv->name, sizeof(priv->name), "axp20x:%s", str);
> +	priv->cdev.name = priv->name;
> +
> +	priv->cdev.default_trigger = of_get_property(np,
> +						     "linux,default-trigger",
> +						     NULL);
> +
> +	if (!of_property_read_u8(np, "x-powers,charger-mode", &value)) {
> +		priv->ctrl = AXP20X_CHGLED_CTRL_CHARGER;
> +		priv->mode = (value < 2) ? value : 0;
> +	} else {
> +		priv->ctrl = AXP20X_CHGLED_CTRL_MANUAL;
> +	}
> +
> +	str = of_get_property(np, "default-state", NULL);
> +	if (str) {
> +		if (!strcmp(str, "keep")) {
> +			ret = axp20x_led_brightness_get(&priv->cdev);
> +			if (ret < 0)
> +				return ret;
> +			priv->cdev.brightness = ret;
> +		} else if (!strcmp(str, "on")) {
> +			ret = axp20x_led_brightness_set_blocking(&priv->cdev,
> +								 LED_FULL);
> +		} else  {
> +			ret = axp20x_led_brightness_set_blocking(&priv->cdev,
> +								 LED_OFF);
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct of_device_id axp20x_led_of_match[] = {
> +	{ .compatible = "x-powers,axp20x-led" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, axp20x_led_of_match);
> +
> +static int axp20x_led_probe(struct platform_device *pdev)
> +{
> +	struct axp20x_led *priv;
> +	int ret;
> +
> +	if (!of_device_is_available(pdev->dev.of_node))
> +		return -ENODEV;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(struct axp20x_led),
> +			    GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->axp20x = dev_get_drvdata(pdev->dev.parent);
> +	if (!priv->axp20x) {
> +		dev_err(&pdev->dev, "Failed to get parent data\n");
> +		return -ENXIO;
> +	}
> +
> +	mutex_init(&priv->lock);
> +
> +	priv->cdev.brightness_set_blocking = axp20x_led_brightness_set_blocking;
> +	priv->cdev.brightness_get = axp20x_led_brightness_get;
> +	priv->cdev.groups = axp20x_led_groups;
> +
> +	ret = axp20x_led_parse_dt(priv, pdev->dev.of_node);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Failed to set parameters\n");
> +		return ret;
> +	}
> +
> +	/**
> +	 * For some reason in AXP209 the bit that controls CHGLED is with
> +	 * inverted logic compared to all other PMICs.
> +	 * If the PMIC is actually AXP209, set inverted flag and later use it
> +	 * when configuring the LED.
> +	 */
> +	if (priv->axp20x->variant == AXP209_ID)
> +		priv->ctrl_inverted = 1;
> +
> +	ret =  axp20x_led_setup(priv);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Failed to configure led");
> +		return ret;
> +	}
> +
> +	return devm_led_classdev_register(&pdev->dev, &priv->cdev);
> +}
> +
> +static struct platform_driver axp20x_led_driver = {
> +	.driver = {
> +		.name	= "axp20x-led",
> +		.of_match_table = of_match_ptr(axp20x_led_of_match),
> +	},
> +	.probe = axp20x_led_probe,
> +};
> +
> +module_platform_driver(axp20x_led_driver);
> +
> +MODULE_AUTHOR("Stefan Mavrodiev <stefan@olimex.com");
> +MODULE_DESCRIPTION("X-Powers PMIC CHGLED driver");
> +MODULE_LICENSE("GPL");

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

* Re: [PATCH v2 1/8] leds: Add support for AXP20X CHGLED
@ 2019-02-15 14:07     ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-15 14:07 UTC (permalink / raw)
  To: Stefan Mavrodiev, Jacek Anaszewski, Pavel Machek, Rob Herring,
	Mark Rutland, Chen-Yu Tsai, Maxime Ripard, Lee Jones,
	open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support


On 2/15/19 1:50 PM, Stefan Mavrodiev wrote:
> Most of AXP20x PMIC chips have built-in battery charger with LED indicator.
> The LED can be controlled ether by the charger or manually by a register.
>
> The default is (except for AXP209) manual control, which makes this LED
> useless, since there is no device driver.
>
> The driver rely on AXP20X MFD driver.
>
> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
> ---
>   drivers/leds/Kconfig       |  10 ++
>   drivers/leds/Makefile      |   1 +
>   drivers/leds/leds-axp20x.c | 291 +++++++++++++++++++++++++++++++++++++
>   3 files changed, 302 insertions(+)
>   create mode 100644 drivers/leds/leds-axp20x.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index a72f97fca57b..82dce9063d41 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -766,6 +766,16 @@ config LEDS_NIC78BX
>   	  To compile this driver as a module, choose M here: the module
>   	  will be called leds-nic78bx.
>   
> +config LEDS_AXP20X
> +	tristate "LED support for X-Powers PMICs"
> +	depends on MFD_AXP20X
> +	help
> +	  This option enables support for CHGLED found on most of X-Powers
> +	  PMICs.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called leds-axp20x.
> +
>   comment "LED Triggers"
>   source "drivers/leds/trigger/Kconfig"
>   
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 4c1b0054f379..d3fb76e119d8 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -79,6 +79,7 @@ obj-$(CONFIG_LEDS_MT6323)		+= leds-mt6323.o
>   obj-$(CONFIG_LEDS_LM3692X)		+= leds-lm3692x.o
>   obj-$(CONFIG_LEDS_SC27XX_BLTC)		+= leds-sc27xx-bltc.o
>   obj-$(CONFIG_LEDS_LM3601X)		+= leds-lm3601x.o
> +obj-$(CONFIG_LEDS_AXP20X)		+= leds-axp20x.o
>   
>   # LED SPI Drivers
>   obj-$(CONFIG_LEDS_CR0014114)		+= leds-cr0014114.o
> diff --git a/drivers/leds/leds-axp20x.c b/drivers/leds/leds-axp20x.c
> new file mode 100644
> index 000000000000..2d5ae1c085f0
> --- /dev/null
> +++ b/drivers/leds/leds-axp20x.c
> @@ -0,0 +1,291 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +//
> +// Copyright 2019 Stefan Mavrodiev <stefan@olimex.com>
> +

The copyright header is wrong. It should be "Copyright 2019 Olimex Ltd.".

I'd like to fix it before the patchis merged (eventually).

Regards,
Stefan Mavrodiev

> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/leds.h>
> +#include <linux/mfd/axp20x.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <uapi/linux/uleds.h>
> +
> +
> +#define AXP20X_CHGLED_CTRL_REG		AXP20X_OFF_CTRL
> +#define AXP20X_CHGLED_FUNC_MASK			GENMASK(5, 4)
> +#define AXP20X_CHGLED_FUNC_OFF			(0 << 4)
> +#define AXP20X_CHGLED_FUNC_1HZ			(1 << 4)
> +#define AXP20X_CHGLED_FUNC_4HZ			(2 << 4)
> +#define AXP20X_CHGLED_FUNC_FULL			(3 << 4)
> +#define AXP20X_CHGLED_CTRL_MASK			BIT(3)
> +#define AXP20X_CHGLED_CTRL_MANUAL		0
> +#define AXP20X_CHGLED_CTRL_CHARGER		1
> +#define AXP20X_CHGLED_CTRL(_ctrl)		(_ctrl << 3)
> +
> +#define AXP20X_CHGLED_MODE_REG		AXP20X_CHRG_CTRL2
> +#define AXP20X_CHGLED_MODE_MASK			BIT(4)
> +#define AXP20X_CHGLED_MODE_A			0
> +#define AXP20X_CHGLED_MODE_B			1
> +#define AXP20X_CHGLED_MODE(_mode)		(_mode << 4)
> +
> +struct axp20x_led {
> +	char			name[LED_MAX_NAME_SIZE];
> +	struct led_classdev	cdev;
> +	struct mutex		lock;
> +	u8			mode : 1;
> +	u8			ctrl : 1;
> +	u8			ctrl_inverted : 1;
> +	struct axp20x_dev	*axp20x;
> +};
> +
> +static inline struct axp20x_led *to_axp20x_led(struct led_classdev *cdev)
> +{
> +	return container_of(cdev, struct axp20x_led, cdev);
> +}
> +
> +static int axp20x_led_setup(struct axp20x_led *priv)
> +{
> +	int ret;
> +	u8 val;
> +
> +	/* Invert the logic, if necessary */
> +	val = priv->ctrl ^ priv->ctrl_inverted;
> +
> +	mutex_lock(&priv->lock);
> +	ret = regmap_update_bits(priv->axp20x->regmap, AXP20X_CHGLED_CTRL_REG,
> +				 AXP20X_CHGLED_CTRL_MASK,
> +				 AXP20X_CHGLED_CTRL(val));
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = regmap_update_bits(priv->axp20x->regmap, AXP20X_CHGLED_MODE_REG,
> +				 AXP20X_CHGLED_MODE_MASK,
> +				 AXP20X_CHGLED_MODE(priv->mode));
> +out:
> +	mutex_unlock(&priv->lock);
> +	return ret;
> +}
> +
> +static ssize_t control_show(struct device *dev, struct device_attribute *attr,
> +			    char *buf)
> +{
> +	struct led_classdev *cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +
> +	return sprintf(buf, "%u\n", priv->ctrl);
> +}
> +
> +static ssize_t control_store(struct device *dev, struct device_attribute *attr,
> +			     const char *buf, size_t size)
> +{
> +	struct led_classdev *cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +	unsigned long val;
> +	int ret;
> +
> +	ret = kstrtoul(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	/**
> +	 * Supported values are:
> +	 *   - 0 : Manual control
> +	 *   - 1 : Charger control
> +	 */
> +	if (val > 1)
> +		return -EINVAL;
> +
> +	priv->ctrl = val;
> +
> +	return axp20x_led_setup(priv) ? : size;
> +}
> +static DEVICE_ATTR_RW(control);
> +
> +static ssize_t mode_show(struct device *dev, struct device_attribute *attr,
> +			 char *buf)
> +{
> +	struct led_classdev *cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +
> +	return sprintf(buf, "%u\n", priv->mode);
> +}
> +
> +static ssize_t mode_store(struct device *dev, struct device_attribute *attr,
> +			  const char *buf, size_t size)
> +{
> +	struct led_classdev *cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +	unsigned long val;
> +	int ret;
> +
> +	ret = kstrtoul(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +	/**
> +	 * Supported values are:
> +	 *   - 0 : Mode A
> +	 *   - 1 : Mode B
> +	 */
> +	if (val > 1)
> +		return -EINVAL;
> +
> +	priv->mode = val;
> +
> +	return axp20x_led_setup(priv) ? : size;
> +}
> +static DEVICE_ATTR_RW(mode);
> +
> +static struct attribute *axp20x_led_attrs[] = {
> +	&dev_attr_control.attr,
> +	&dev_attr_mode.attr,
> +	NULL,
> +};
> +ATTRIBUTE_GROUPS(axp20x_led);
> +
> +enum led_brightness axp20x_led_brightness_get(struct led_classdev *cdev)
> +{
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +	u32 val;
> +	int ret;
> +
> +	mutex_lock(&priv->lock);
> +	ret = regmap_read(priv->axp20x->regmap, AXP20X_CHGLED_CTRL_REG, &val);
> +	mutex_unlock(&priv->lock);
> +	if (ret < 0)
> +		return LED_OFF;
> +
> +	return (val & AXP20X_CHGLED_FUNC_FULL) ? LED_FULL : LED_OFF;
> +}
> +
> +static int axp20x_led_brightness_set_blocking(struct led_classdev *cdev,
> +					      enum led_brightness brightness)
> +{
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +	int ret = 0;
> +
> +	mutex_lock(&priv->lock);
> +	ret = regmap_update_bits(priv->axp20x->regmap,
> +				 AXP20X_CHGLED_CTRL_REG,
> +				 AXP20X_CHGLED_FUNC_MASK,
> +				 (brightness) ?
> +				 AXP20X_CHGLED_FUNC_FULL :
> +				 AXP20X_CHGLED_FUNC_OFF);
> +	mutex_unlock(&priv->lock);
> +
> +	return ret;
> +}
> +
> +static int axp20x_led_parse_dt(struct axp20x_led *priv, struct device_node *np)
> +{
> +	const char *str;
> +	u8 value;
> +	int ret = 0;
> +
> +	str = of_get_property(np, "label", NULL);
> +	if (!str)
> +		snprintf(priv->name, sizeof(priv->name), "axp20x::");
> +	else
> +		snprintf(priv->name, sizeof(priv->name), "axp20x:%s", str);
> +	priv->cdev.name = priv->name;
> +
> +	priv->cdev.default_trigger = of_get_property(np,
> +						     "linux,default-trigger",
> +						     NULL);
> +
> +	if (!of_property_read_u8(np, "x-powers,charger-mode", &value)) {
> +		priv->ctrl = AXP20X_CHGLED_CTRL_CHARGER;
> +		priv->mode = (value < 2) ? value : 0;
> +	} else {
> +		priv->ctrl = AXP20X_CHGLED_CTRL_MANUAL;
> +	}
> +
> +	str = of_get_property(np, "default-state", NULL);
> +	if (str) {
> +		if (!strcmp(str, "keep")) {
> +			ret = axp20x_led_brightness_get(&priv->cdev);
> +			if (ret < 0)
> +				return ret;
> +			priv->cdev.brightness = ret;
> +		} else if (!strcmp(str, "on")) {
> +			ret = axp20x_led_brightness_set_blocking(&priv->cdev,
> +								 LED_FULL);
> +		} else  {
> +			ret = axp20x_led_brightness_set_blocking(&priv->cdev,
> +								 LED_OFF);
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct of_device_id axp20x_led_of_match[] = {
> +	{ .compatible = "x-powers,axp20x-led" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, axp20x_led_of_match);
> +
> +static int axp20x_led_probe(struct platform_device *pdev)
> +{
> +	struct axp20x_led *priv;
> +	int ret;
> +
> +	if (!of_device_is_available(pdev->dev.of_node))
> +		return -ENODEV;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(struct axp20x_led),
> +			    GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->axp20x = dev_get_drvdata(pdev->dev.parent);
> +	if (!priv->axp20x) {
> +		dev_err(&pdev->dev, "Failed to get parent data\n");
> +		return -ENXIO;
> +	}
> +
> +	mutex_init(&priv->lock);
> +
> +	priv->cdev.brightness_set_blocking = axp20x_led_brightness_set_blocking;
> +	priv->cdev.brightness_get = axp20x_led_brightness_get;
> +	priv->cdev.groups = axp20x_led_groups;
> +
> +	ret = axp20x_led_parse_dt(priv, pdev->dev.of_node);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Failed to set parameters\n");
> +		return ret;
> +	}
> +
> +	/**
> +	 * For some reason in AXP209 the bit that controls CHGLED is with
> +	 * inverted logic compared to all other PMICs.
> +	 * If the PMIC is actually AXP209, set inverted flag and later use it
> +	 * when configuring the LED.
> +	 */
> +	if (priv->axp20x->variant == AXP209_ID)
> +		priv->ctrl_inverted = 1;
> +
> +	ret =  axp20x_led_setup(priv);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Failed to configure led");
> +		return ret;
> +	}
> +
> +	return devm_led_classdev_register(&pdev->dev, &priv->cdev);
> +}
> +
> +static struct platform_driver axp20x_led_driver = {
> +	.driver = {
> +		.name	= "axp20x-led",
> +		.of_match_table = of_match_ptr(axp20x_led_of_match),
> +	},
> +	.probe = axp20x_led_probe,
> +};
> +
> +module_platform_driver(axp20x_led_driver);
> +
> +MODULE_AUTHOR("Stefan Mavrodiev <stefan@olimex.com");
> +MODULE_DESCRIPTION("X-Powers PMIC CHGLED driver");
> +MODULE_LICENSE("GPL");

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

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

* Re: [PATCH v2 1/8] leds: Add support for AXP20X CHGLED
  2019-02-15 11:50   ` Stefan Mavrodiev
  (?)
@ 2019-02-15 15:57     ` Maxime Ripard
  -1 siblings, 0 replies; 54+ messages in thread
From: Maxime Ripard @ 2019-02-15 15:57 UTC (permalink / raw)
  To: Stefan Mavrodiev
  Cc: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support

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

Hi Stefan,

Thanks for working on this.

On Fri, Feb 15, 2019 at 01:50:06PM +0200, Stefan Mavrodiev wrote:
> +static ssize_t control_show(struct device *dev, struct device_attribute *attr,
> +			    char *buf)
> +{
> +	struct led_classdev *cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +
> +	return sprintf(buf, "%u\n", priv->ctrl);
> +}
> +
> +static ssize_t control_store(struct device *dev, struct device_attribute *attr,
> +			     const char *buf, size_t size)
> +{
> +	struct led_classdev *cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +	unsigned long val;
> +	int ret;
> +
> +	ret = kstrtoul(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	/**
> +	 * Supported values are:
> +	 *   - 0 : Manual control
> +	 *   - 1 : Charger control
> +	 */
> +	if (val > 1)
> +		return -EINVAL;
> +
> +	priv->ctrl = val;
> +
> +	return axp20x_led_setup(priv) ? : size;
> +}
> +static DEVICE_ATTR_RW(control);
> +
> +static ssize_t mode_show(struct device *dev, struct device_attribute *attr,
> +			 char *buf)
> +{
> +	struct led_classdev *cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +
> +	return sprintf(buf, "%u\n", priv->mode);
> +}
> +
> +static ssize_t mode_store(struct device *dev, struct device_attribute *attr,
> +			  const char *buf, size_t size)
> +{
> +	struct led_classdev *cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +	unsigned long val;
> +	int ret;
> +
> +	ret = kstrtoul(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +	/**
> +	 * Supported values are:
> +	 *   - 0 : Mode A
> +	 *   - 1 : Mode B
> +	 */
> +	if (val > 1)
> +		return -EINVAL;
> +
> +	priv->mode = val;
> +
> +	return axp20x_led_setup(priv) ? : size;
> +}
> +static DEVICE_ATTR_RW(mode);
> +
> +static struct attribute *axp20x_led_attrs[] = {
> +	&dev_attr_control.attr,
> +	&dev_attr_mode.attr,
> +	NULL,
> +};
> +ATTRIBUTE_GROUPS(axp20x_led);

I can't really say whether adding sysfs handles for this is the right
thing to do, but if it is you should document the interface.

> +	if (!of_property_read_u8(np, "x-powers,charger-mode", &value)) {
> +		priv->ctrl = AXP20X_CHGLED_CTRL_CHARGER;
> +		priv->mode = (value < 2) ? value : 0;
> +	} else {
> +		priv->ctrl = AXP20X_CHGLED_CTRL_MANUAL;
> +	}

I'm not sure we want to make this a property of the device
tree. Changing the device tree isn't an option for some users, so we
need to make sure we can change it even if we can't change the device
tree.

A kernel module is one option, but the other reviewers might have some
alternatives.

> +	str = of_get_property(np, "default-state", NULL);
> +	if (str) {
> +		if (!strcmp(str, "keep")) {
> +			ret = axp20x_led_brightness_get(&priv->cdev);
> +			if (ret < 0)
> +				return ret;
> +			priv->cdev.brightness = ret;
> +		} else if (!strcmp(str, "on")) {
> +			ret = axp20x_led_brightness_set_blocking(&priv->cdev,
> +								 LED_FULL);
> +		} else  {
> +			ret = axp20x_led_brightness_set_blocking(&priv->cdev,
> +								 LED_OFF);
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct of_device_id axp20x_led_of_match[] = {
> +	{ .compatible = "x-powers,axp20x-led" },

Having a wildcard in the compatible isn't great, since it doesn't
really allow you to identify which part it is you're actually using,
and that wildcard might become inconsistent at some point.

You'd better use axp209-led (but the name of the driver is fine).

> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, axp20x_led_of_match);
> +
> +static int axp20x_led_probe(struct platform_device *pdev)
> +{
> +	struct axp20x_led *priv;
> +	int ret;
> +
> +	if (!of_device_is_available(pdev->dev.of_node))
> +		return -ENODEV;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(struct axp20x_led),
> +			    GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->axp20x = dev_get_drvdata(pdev->dev.parent);
> +	if (!priv->axp20x) {
> +		dev_err(&pdev->dev, "Failed to get parent data\n");
> +		return -ENXIO;
> +	}
> +
> +	mutex_init(&priv->lock);
> +
> +	priv->cdev.brightness_set_blocking = axp20x_led_brightness_set_blocking;
> +	priv->cdev.brightness_get = axp20x_led_brightness_get;
> +	priv->cdev.groups = axp20x_led_groups;
> +
> +	ret = axp20x_led_parse_dt(priv, pdev->dev.of_node);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Failed to set parameters\n");
> +		return ret;
> +	}
> +
> +	/**
> +	 * For some reason in AXP209 the bit that controls CHGLED is with
> +	 * inverted logic compared to all other PMICs.
> +	 * If the PMIC is actually AXP209, set inverted flag and later use it
> +	 * when configuring the LED.
> +	 */
> +	if (priv->axp20x->variant == AXP209_ID)
> +		priv->ctrl_inverted = 1;

This should be matched on the compatible.

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

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

* Re: [PATCH v2 1/8] leds: Add support for AXP20X CHGLED
@ 2019-02-15 15:57     ` Maxime Ripard
  0 siblings, 0 replies; 54+ messages in thread
From: Maxime Ripard @ 2019-02-15 15:57 UTC (permalink / raw)
  To: Stefan Mavrodiev
  Cc: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support

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

Hi Stefan,

Thanks for working on this.

On Fri, Feb 15, 2019 at 01:50:06PM +0200, Stefan Mavrodiev wrote:
> +static ssize_t control_show(struct device *dev, struct device_attribute *attr,
> +			    char *buf)
> +{
> +	struct led_classdev *cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +
> +	return sprintf(buf, "%u\n", priv->ctrl);
> +}
> +
> +static ssize_t control_store(struct device *dev, struct device_attribute *attr,
> +			     const char *buf, size_t size)
> +{
> +	struct led_classdev *cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +	unsigned long val;
> +	int ret;
> +
> +	ret = kstrtoul(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	/**
> +	 * Supported values are:
> +	 *   - 0 : Manual control
> +	 *   - 1 : Charger control
> +	 */
> +	if (val > 1)
> +		return -EINVAL;
> +
> +	priv->ctrl = val;
> +
> +	return axp20x_led_setup(priv) ? : size;
> +}
> +static DEVICE_ATTR_RW(control);
> +
> +static ssize_t mode_show(struct device *dev, struct device_attribute *attr,
> +			 char *buf)
> +{
> +	struct led_classdev *cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +
> +	return sprintf(buf, "%u\n", priv->mode);
> +}
> +
> +static ssize_t mode_store(struct device *dev, struct device_attribute *attr,
> +			  const char *buf, size_t size)
> +{
> +	struct led_classdev *cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +	unsigned long val;
> +	int ret;
> +
> +	ret = kstrtoul(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +	/**
> +	 * Supported values are:
> +	 *   - 0 : Mode A
> +	 *   - 1 : Mode B
> +	 */
> +	if (val > 1)
> +		return -EINVAL;
> +
> +	priv->mode = val;
> +
> +	return axp20x_led_setup(priv) ? : size;
> +}
> +static DEVICE_ATTR_RW(mode);
> +
> +static struct attribute *axp20x_led_attrs[] = {
> +	&dev_attr_control.attr,
> +	&dev_attr_mode.attr,
> +	NULL,
> +};
> +ATTRIBUTE_GROUPS(axp20x_led);

I can't really say whether adding sysfs handles for this is the right
thing to do, but if it is you should document the interface.

> +	if (!of_property_read_u8(np, "x-powers,charger-mode", &value)) {
> +		priv->ctrl = AXP20X_CHGLED_CTRL_CHARGER;
> +		priv->mode = (value < 2) ? value : 0;
> +	} else {
> +		priv->ctrl = AXP20X_CHGLED_CTRL_MANUAL;
> +	}

I'm not sure we want to make this a property of the device
tree. Changing the device tree isn't an option for some users, so we
need to make sure we can change it even if we can't change the device
tree.

A kernel module is one option, but the other reviewers might have some
alternatives.

> +	str = of_get_property(np, "default-state", NULL);
> +	if (str) {
> +		if (!strcmp(str, "keep")) {
> +			ret = axp20x_led_brightness_get(&priv->cdev);
> +			if (ret < 0)
> +				return ret;
> +			priv->cdev.brightness = ret;
> +		} else if (!strcmp(str, "on")) {
> +			ret = axp20x_led_brightness_set_blocking(&priv->cdev,
> +								 LED_FULL);
> +		} else  {
> +			ret = axp20x_led_brightness_set_blocking(&priv->cdev,
> +								 LED_OFF);
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct of_device_id axp20x_led_of_match[] = {
> +	{ .compatible = "x-powers,axp20x-led" },

Having a wildcard in the compatible isn't great, since it doesn't
really allow you to identify which part it is you're actually using,
and that wildcard might become inconsistent at some point.

You'd better use axp209-led (but the name of the driver is fine).

> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, axp20x_led_of_match);
> +
> +static int axp20x_led_probe(struct platform_device *pdev)
> +{
> +	struct axp20x_led *priv;
> +	int ret;
> +
> +	if (!of_device_is_available(pdev->dev.of_node))
> +		return -ENODEV;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(struct axp20x_led),
> +			    GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->axp20x = dev_get_drvdata(pdev->dev.parent);
> +	if (!priv->axp20x) {
> +		dev_err(&pdev->dev, "Failed to get parent data\n");
> +		return -ENXIO;
> +	}
> +
> +	mutex_init(&priv->lock);
> +
> +	priv->cdev.brightness_set_blocking = axp20x_led_brightness_set_blocking;
> +	priv->cdev.brightness_get = axp20x_led_brightness_get;
> +	priv->cdev.groups = axp20x_led_groups;
> +
> +	ret = axp20x_led_parse_dt(priv, pdev->dev.of_node);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Failed to set parameters\n");
> +		return ret;
> +	}
> +
> +	/**
> +	 * For some reason in AXP209 the bit that controls CHGLED is with
> +	 * inverted logic compared to all other PMICs.
> +	 * If the PMIC is actually AXP209, set inverted flag and later use it
> +	 * when configuring the LED.
> +	 */
> +	if (priv->axp20x->variant == AXP209_ID)
> +		priv->ctrl_inverted = 1;

This should be matched on the compatible.

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

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

* Re: [PATCH v2 1/8] leds: Add support for AXP20X CHGLED
@ 2019-02-15 15:57     ` Maxime Ripard
  0 siblings, 0 replies; 54+ messages in thread
From: Maxime Ripard @ 2019-02-15 15:57 UTC (permalink / raw)
  To: Stefan Mavrodiev
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	Chen-Yu Tsai, Rob Herring, Jacek Anaszewski, Pavel Machek,
	Lee Jones, open list:LED SUBSYSTEM,
	moderated list:ARM/Allwinner sunXi SoC support


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

Hi Stefan,

Thanks for working on this.

On Fri, Feb 15, 2019 at 01:50:06PM +0200, Stefan Mavrodiev wrote:
> +static ssize_t control_show(struct device *dev, struct device_attribute *attr,
> +			    char *buf)
> +{
> +	struct led_classdev *cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +
> +	return sprintf(buf, "%u\n", priv->ctrl);
> +}
> +
> +static ssize_t control_store(struct device *dev, struct device_attribute *attr,
> +			     const char *buf, size_t size)
> +{
> +	struct led_classdev *cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +	unsigned long val;
> +	int ret;
> +
> +	ret = kstrtoul(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	/**
> +	 * Supported values are:
> +	 *   - 0 : Manual control
> +	 *   - 1 : Charger control
> +	 */
> +	if (val > 1)
> +		return -EINVAL;
> +
> +	priv->ctrl = val;
> +
> +	return axp20x_led_setup(priv) ? : size;
> +}
> +static DEVICE_ATTR_RW(control);
> +
> +static ssize_t mode_show(struct device *dev, struct device_attribute *attr,
> +			 char *buf)
> +{
> +	struct led_classdev *cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +
> +	return sprintf(buf, "%u\n", priv->mode);
> +}
> +
> +static ssize_t mode_store(struct device *dev, struct device_attribute *attr,
> +			  const char *buf, size_t size)
> +{
> +	struct led_classdev *cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *priv = to_axp20x_led(cdev);
> +	unsigned long val;
> +	int ret;
> +
> +	ret = kstrtoul(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +	/**
> +	 * Supported values are:
> +	 *   - 0 : Mode A
> +	 *   - 1 : Mode B
> +	 */
> +	if (val > 1)
> +		return -EINVAL;
> +
> +	priv->mode = val;
> +
> +	return axp20x_led_setup(priv) ? : size;
> +}
> +static DEVICE_ATTR_RW(mode);
> +
> +static struct attribute *axp20x_led_attrs[] = {
> +	&dev_attr_control.attr,
> +	&dev_attr_mode.attr,
> +	NULL,
> +};
> +ATTRIBUTE_GROUPS(axp20x_led);

I can't really say whether adding sysfs handles for this is the right
thing to do, but if it is you should document the interface.

> +	if (!of_property_read_u8(np, "x-powers,charger-mode", &value)) {
> +		priv->ctrl = AXP20X_CHGLED_CTRL_CHARGER;
> +		priv->mode = (value < 2) ? value : 0;
> +	} else {
> +		priv->ctrl = AXP20X_CHGLED_CTRL_MANUAL;
> +	}

I'm not sure we want to make this a property of the device
tree. Changing the device tree isn't an option for some users, so we
need to make sure we can change it even if we can't change the device
tree.

A kernel module is one option, but the other reviewers might have some
alternatives.

> +	str = of_get_property(np, "default-state", NULL);
> +	if (str) {
> +		if (!strcmp(str, "keep")) {
> +			ret = axp20x_led_brightness_get(&priv->cdev);
> +			if (ret < 0)
> +				return ret;
> +			priv->cdev.brightness = ret;
> +		} else if (!strcmp(str, "on")) {
> +			ret = axp20x_led_brightness_set_blocking(&priv->cdev,
> +								 LED_FULL);
> +		} else  {
> +			ret = axp20x_led_brightness_set_blocking(&priv->cdev,
> +								 LED_OFF);
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct of_device_id axp20x_led_of_match[] = {
> +	{ .compatible = "x-powers,axp20x-led" },

Having a wildcard in the compatible isn't great, since it doesn't
really allow you to identify which part it is you're actually using,
and that wildcard might become inconsistent at some point.

You'd better use axp209-led (but the name of the driver is fine).

> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, axp20x_led_of_match);
> +
> +static int axp20x_led_probe(struct platform_device *pdev)
> +{
> +	struct axp20x_led *priv;
> +	int ret;
> +
> +	if (!of_device_is_available(pdev->dev.of_node))
> +		return -ENODEV;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(struct axp20x_led),
> +			    GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->axp20x = dev_get_drvdata(pdev->dev.parent);
> +	if (!priv->axp20x) {
> +		dev_err(&pdev->dev, "Failed to get parent data\n");
> +		return -ENXIO;
> +	}
> +
> +	mutex_init(&priv->lock);
> +
> +	priv->cdev.brightness_set_blocking = axp20x_led_brightness_set_blocking;
> +	priv->cdev.brightness_get = axp20x_led_brightness_get;
> +	priv->cdev.groups = axp20x_led_groups;
> +
> +	ret = axp20x_led_parse_dt(priv, pdev->dev.of_node);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "Failed to set parameters\n");
> +		return ret;
> +	}
> +
> +	/**
> +	 * For some reason in AXP209 the bit that controls CHGLED is with
> +	 * inverted logic compared to all other PMICs.
> +	 * If the PMIC is actually AXP209, set inverted flag and later use it
> +	 * when configuring the LED.
> +	 */
> +	if (priv->axp20x->variant == AXP209_ID)
> +		priv->ctrl_inverted = 1;

This should be matched on the compatible.

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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

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

* Re: [PATCH v2 1/8] leds: Add support for AXP20X CHGLED
  2019-02-15 15:57     ` Maxime Ripard
  (?)
@ 2019-02-15 18:32       ` Pavel Machek
  -1 siblings, 0 replies; 54+ messages in thread
From: Pavel Machek @ 2019-02-15 18:32 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Stefan Mavrodiev, Jacek Anaszewski, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support

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

Hi!

> On Fri, Feb 15, 2019 at 01:50:06PM +0200, Stefan Mavrodiev wrote:

> > +static ssize_t control_store(struct device *dev, struct device_attribute *attr,
> > +			     const char *buf, size_t size)
> > +{
> > +	struct led_classdev *cdev = dev_get_drvdata(dev);
> > +	struct axp20x_led *priv = to_axp20x_led(cdev);
> > +	unsigned long val;
> > +	int ret;
> > +
> > +	ret = kstrtoul(buf, 0, &val);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/**
> > +	 * Supported values are:
> > +	 *   - 0 : Manual control
> > +	 *   - 1 : Charger control
> > +	 */
...
> > +static struct attribute *axp20x_led_attrs[] = {
> > +	&dev_attr_control.attr,
> > +	&dev_attr_mode.attr,
> > +	NULL,
> > +};
> > +ATTRIBUTE_GROUPS(axp20x_led);
> 
> I can't really say whether adding sysfs handles for this is the right
> thing to do, but if it is you should document the interface.

It is not. See "Add Intel Cherry Trail Whiskey Cove PMIC LEDs" thread
in the last few days.

> > +	if (!of_property_read_u8(np, "x-powers,charger-mode", &value)) {
> > +		priv->ctrl = AXP20X_CHGLED_CTRL_CHARGER;
> > +		priv->mode = (value < 2) ? value : 0;
> > +	} else {
> > +		priv->ctrl = AXP20X_CHGLED_CTRL_MANUAL;
> > +	}
> 
> I'm not sure we want to make this a property of the device
> tree. Changing the device tree isn't an option for some users, so we
> need to make sure we can change it even if we can't change the device
> tree.

We want this to be configurable at run time. It can get default from
the device tree.

If we go for the "hardware" trigger, you'll get it for free.

Best regards,
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

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

* Re: [PATCH v2 1/8] leds: Add support for AXP20X CHGLED
@ 2019-02-15 18:32       ` Pavel Machek
  0 siblings, 0 replies; 54+ messages in thread
From: Pavel Machek @ 2019-02-15 18:32 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Stefan Mavrodiev, Jacek Anaszewski, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support

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

Hi!

> On Fri, Feb 15, 2019 at 01:50:06PM +0200, Stefan Mavrodiev wrote:

> > +static ssize_t control_store(struct device *dev, struct device_attribute *attr,
> > +			     const char *buf, size_t size)
> > +{
> > +	struct led_classdev *cdev = dev_get_drvdata(dev);
> > +	struct axp20x_led *priv = to_axp20x_led(cdev);
> > +	unsigned long val;
> > +	int ret;
> > +
> > +	ret = kstrtoul(buf, 0, &val);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/**
> > +	 * Supported values are:
> > +	 *   - 0 : Manual control
> > +	 *   - 1 : Charger control
> > +	 */
...
> > +static struct attribute *axp20x_led_attrs[] = {
> > +	&dev_attr_control.attr,
> > +	&dev_attr_mode.attr,
> > +	NULL,
> > +};
> > +ATTRIBUTE_GROUPS(axp20x_led);
> 
> I can't really say whether adding sysfs handles for this is the right
> thing to do, but if it is you should document the interface.

It is not. See "Add Intel Cherry Trail Whiskey Cove PMIC LEDs" thread
in the last few days.

> > +	if (!of_property_read_u8(np, "x-powers,charger-mode", &value)) {
> > +		priv->ctrl = AXP20X_CHGLED_CTRL_CHARGER;
> > +		priv->mode = (value < 2) ? value : 0;
> > +	} else {
> > +		priv->ctrl = AXP20X_CHGLED_CTRL_MANUAL;
> > +	}
> 
> I'm not sure we want to make this a property of the device
> tree. Changing the device tree isn't an option for some users, so we
> need to make sure we can change it even if we can't change the device
> tree.

We want this to be configurable at run time. It can get default from
the device tree.

If we go for the "hardware" trigger, you'll get it for free.

Best regards,
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

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

* Re: [PATCH v2 1/8] leds: Add support for AXP20X CHGLED
@ 2019-02-15 18:32       ` Pavel Machek
  0 siblings, 0 replies; 54+ messages in thread
From: Pavel Machek @ 2019-02-15 18:32 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Stefan Mavrodiev,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	Chen-Yu Tsai, Rob Herring, Jacek Anaszewski, Lee Jones,
	open list:LED SUBSYSTEM,
	moderated list:ARM/Allwinner sunXi SoC support


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

Hi!

> On Fri, Feb 15, 2019 at 01:50:06PM +0200, Stefan Mavrodiev wrote:

> > +static ssize_t control_store(struct device *dev, struct device_attribute *attr,
> > +			     const char *buf, size_t size)
> > +{
> > +	struct led_classdev *cdev = dev_get_drvdata(dev);
> > +	struct axp20x_led *priv = to_axp20x_led(cdev);
> > +	unsigned long val;
> > +	int ret;
> > +
> > +	ret = kstrtoul(buf, 0, &val);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/**
> > +	 * Supported values are:
> > +	 *   - 0 : Manual control
> > +	 *   - 1 : Charger control
> > +	 */
...
> > +static struct attribute *axp20x_led_attrs[] = {
> > +	&dev_attr_control.attr,
> > +	&dev_attr_mode.attr,
> > +	NULL,
> > +};
> > +ATTRIBUTE_GROUPS(axp20x_led);
> 
> I can't really say whether adding sysfs handles for this is the right
> thing to do, but if it is you should document the interface.

It is not. See "Add Intel Cherry Trail Whiskey Cove PMIC LEDs" thread
in the last few days.

> > +	if (!of_property_read_u8(np, "x-powers,charger-mode", &value)) {
> > +		priv->ctrl = AXP20X_CHGLED_CTRL_CHARGER;
> > +		priv->mode = (value < 2) ? value : 0;
> > +	} else {
> > +		priv->ctrl = AXP20X_CHGLED_CTRL_MANUAL;
> > +	}
> 
> I'm not sure we want to make this a property of the device
> tree. Changing the device tree isn't an option for some users, so we
> need to make sure we can change it even if we can't change the device
> tree.

We want this to be configurable at run time. It can get default from
the device tree.

If we go for the "hardware" trigger, you'll get it for free.

Best regards,
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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

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

* Re: [PATCH v2 5/8] arm64: dts: allwinner: Enable AXP803 CHGLED for Olimex boards
  2019-02-15 11:50   ` Stefan Mavrodiev
  (?)
@ 2019-02-15 18:49     ` Pavel Machek
  -1 siblings, 0 replies; 54+ messages in thread
From: Pavel Machek @ 2019-02-15 18:49 UTC (permalink / raw)
  To: Stefan Mavrodiev
  Cc: Jacek Anaszewski, Rob Herring, Mark Rutland, Chen-Yu Tsai,
	Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support

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

On Fri 2019-02-15 13:50:10, Stefan Mavrodiev wrote:
> Teres-I and A64-OLinuXino commes with populated LED connected
> to AXP803. So to be used as battery indication, pass the LED control
> to the charger itself in mode 0 ( FULL while charging, OFF no battery
> or completed).
> 
> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts | 6 ++++++
>  arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts   | 6 ++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
> index f7a4bccaa5d4..7d6930319fba 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
> @@ -177,6 +177,12 @@
>  
>  #include "axp803.dtsi"
>  
> +&axp_led {
> +	label = "a64-olinuxino:yellow:chgled";

There's little chance userspace will recognize this led.

Can you make it "platform:yellow:charging" and fix it for other
boards, too?

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

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

* Re: [PATCH v2 5/8] arm64: dts: allwinner: Enable AXP803 CHGLED for Olimex boards
@ 2019-02-15 18:49     ` Pavel Machek
  0 siblings, 0 replies; 54+ messages in thread
From: Pavel Machek @ 2019-02-15 18:49 UTC (permalink / raw)
  To: Stefan Mavrodiev
  Cc: Jacek Anaszewski, Rob Herring, Mark Rutland, Chen-Yu Tsai,
	Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support

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

On Fri 2019-02-15 13:50:10, Stefan Mavrodiev wrote:
> Teres-I and A64-OLinuXino commes with populated LED connected
> to AXP803. So to be used as battery indication, pass the LED control
> to the charger itself in mode 0 ( FULL while charging, OFF no battery
> or completed).
> 
> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts | 6 ++++++
>  arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts   | 6 ++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
> index f7a4bccaa5d4..7d6930319fba 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
> @@ -177,6 +177,12 @@
>  
>  #include "axp803.dtsi"
>  
> +&axp_led {
> +	label = "a64-olinuxino:yellow:chgled";

There's little chance userspace will recognize this led.

Can you make it "platform:yellow:charging" and fix it for other
boards, too?

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

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

* Re: [PATCH v2 5/8] arm64: dts: allwinner: Enable AXP803 CHGLED for Olimex boards
@ 2019-02-15 18:49     ` Pavel Machek
  0 siblings, 0 replies; 54+ messages in thread
From: Pavel Machek @ 2019-02-15 18:49 UTC (permalink / raw)
  To: Stefan Mavrodiev
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Maxime Ripard,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	Chen-Yu Tsai, Rob Herring, Jacek Anaszewski, Lee Jones,
	open list:LED SUBSYSTEM,
	moderated list:ARM/Allwinner sunXi SoC support


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

On Fri 2019-02-15 13:50:10, Stefan Mavrodiev wrote:
> Teres-I and A64-OLinuXino commes with populated LED connected
> to AXP803. So to be used as battery indication, pass the LED control
> to the charger itself in mode 0 ( FULL while charging, OFF no battery
> or completed).
> 
> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts | 6 ++++++
>  arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts   | 6 ++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
> index f7a4bccaa5d4..7d6930319fba 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
> @@ -177,6 +177,12 @@
>  
>  #include "axp803.dtsi"
>  
> +&axp_led {
> +	label = "a64-olinuxino:yellow:chgled";

There's little chance userspace will recognize this led.

Can you make it "platform:yellow:charging" and fix it for other
boards, too?

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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

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

* Re: [PATCH v2 1/8] leds: Add support for AXP20X CHGLED
  2019-02-15 18:32       ` Pavel Machek
  (?)
@ 2019-02-19  7:42         ` Stefan Mavrodiev
  -1 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-19  7:42 UTC (permalink / raw)
  To: Pavel Machek, Maxime Ripard
  Cc: Stefan Mavrodiev, Jacek Anaszewski, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support


On 2/15/19 8:32 PM, Pavel Machek wrote:
> Hi!
>
>> On Fri, Feb 15, 2019 at 01:50:06PM +0200, Stefan Mavrodiev wrote:
>>> +static ssize_t control_store(struct device *dev, struct device_attribute *attr,
>>> +			     const char *buf, size_t size)
>>> +{
>>> +	struct led_classdev *cdev = dev_get_drvdata(dev);
>>> +	struct axp20x_led *priv = to_axp20x_led(cdev);
>>> +	unsigned long val;
>>> +	int ret;
>>> +
>>> +	ret = kstrtoul(buf, 0, &val);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	/**
>>> +	 * Supported values are:
>>> +	 *   - 0 : Manual control
>>> +	 *   - 1 : Charger control
>>> +	 */
> ...
>>> +static struct attribute *axp20x_led_attrs[] = {
>>> +	&dev_attr_control.attr,
>>> +	&dev_attr_mode.attr,
>>> +	NULL,
>>> +};
>>> +ATTRIBUTE_GROUPS(axp20x_led);
>> I can't really say whether adding sysfs handles for this is the right
>> thing to do, but if it is you should document the interface.
> It is not. See "Add Intel Cherry Trail Whiskey Cove PMIC LEDs" thread
> in the last few days.

What I understood is that there will be changes in the LED core, exporting
a new sysfs entry "hw_control". Then I should set HW_CONTROL flag for the
device and add hw_control_get/set callback functions. And this can happen
when the LED core is updated. Right?

However I didn't understand how (in my case) hardware controlled pattern
will be selected?

Best regards,
Stefan Mavrodiev

>>> +	if (!of_property_read_u8(np, "x-powers,charger-mode", &value)) {
>>> +		priv->ctrl = AXP20X_CHGLED_CTRL_CHARGER;
>>> +		priv->mode = (value < 2) ? value : 0;
>>> +	} else {
>>> +		priv->ctrl = AXP20X_CHGLED_CTRL_MANUAL;
>>> +	}
>> I'm not sure we want to make this a property of the device
>> tree. Changing the device tree isn't an option for some users, so we
>> need to make sure we can change it even if we can't change the device
>> tree.
> We want this to be configurable at run time. It can get default from
> the device tree.
>
> If we go for the "hardware" trigger, you'll get it for free.
>
> Best regards,
> 								Pavel

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

* Re: [PATCH v2 1/8] leds: Add support for AXP20X CHGLED
@ 2019-02-19  7:42         ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-19  7:42 UTC (permalink / raw)
  To: Pavel Machek, Maxime Ripard
  Cc: Stefan Mavrodiev, Jacek Anaszewski, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support


On 2/15/19 8:32 PM, Pavel Machek wrote:
> Hi!
>
>> On Fri, Feb 15, 2019 at 01:50:06PM +0200, Stefan Mavrodiev wrote:
>>> +static ssize_t control_store(struct device *dev, struct device_attribute *attr,
>>> +			     const char *buf, size_t size)
>>> +{
>>> +	struct led_classdev *cdev = dev_get_drvdata(dev);
>>> +	struct axp20x_led *priv = to_axp20x_led(cdev);
>>> +	unsigned long val;
>>> +	int ret;
>>> +
>>> +	ret = kstrtoul(buf, 0, &val);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	/**
>>> +	 * Supported values are:
>>> +	 *   - 0 : Manual control
>>> +	 *   - 1 : Charger control
>>> +	 */
> ...
>>> +static struct attribute *axp20x_led_attrs[] = {
>>> +	&dev_attr_control.attr,
>>> +	&dev_attr_mode.attr,
>>> +	NULL,
>>> +};
>>> +ATTRIBUTE_GROUPS(axp20x_led);
>> I can't really say whether adding sysfs handles for this is the right
>> thing to do, but if it is you should document the interface.
> It is not. See "Add Intel Cherry Trail Whiskey Cove PMIC LEDs" thread
> in the last few days.

What I understood is that there will be changes in the LED core, exporting
a new sysfs entry "hw_control". Then I should set HW_CONTROL flag for the
device and add hw_control_get/set callback functions. And this can happen
when the LED core is updated. Right?

However I didn't understand how (in my case) hardware controlled pattern
will be selected?

Best regards,
Stefan Mavrodiev

>>> +	if (!of_property_read_u8(np, "x-powers,charger-mode", &value)) {
>>> +		priv->ctrl = AXP20X_CHGLED_CTRL_CHARGER;
>>> +		priv->mode = (value < 2) ? value : 0;
>>> +	} else {
>>> +		priv->ctrl = AXP20X_CHGLED_CTRL_MANUAL;
>>> +	}
>> I'm not sure we want to make this a property of the device
>> tree. Changing the device tree isn't an option for some users, so we
>> need to make sure we can change it even if we can't change the device
>> tree.
> We want this to be configurable at run time. It can get default from
> the device tree.
>
> If we go for the "hardware" trigger, you'll get it for free.
>
> Best regards,
> 								Pavel

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

* Re: [PATCH v2 1/8] leds: Add support for AXP20X CHGLED
@ 2019-02-19  7:42         ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-19  7:42 UTC (permalink / raw)
  To: Pavel Machek, Maxime Ripard
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Stefan Mavrodiev,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	Chen-Yu Tsai, Rob Herring, Jacek Anaszewski, Lee Jones,
	open list:LED SUBSYSTEM,
	moderated list:ARM/Allwinner sunXi SoC support


On 2/15/19 8:32 PM, Pavel Machek wrote:
> Hi!
>
>> On Fri, Feb 15, 2019 at 01:50:06PM +0200, Stefan Mavrodiev wrote:
>>> +static ssize_t control_store(struct device *dev, struct device_attribute *attr,
>>> +			     const char *buf, size_t size)
>>> +{
>>> +	struct led_classdev *cdev = dev_get_drvdata(dev);
>>> +	struct axp20x_led *priv = to_axp20x_led(cdev);
>>> +	unsigned long val;
>>> +	int ret;
>>> +
>>> +	ret = kstrtoul(buf, 0, &val);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	/**
>>> +	 * Supported values are:
>>> +	 *   - 0 : Manual control
>>> +	 *   - 1 : Charger control
>>> +	 */
> ...
>>> +static struct attribute *axp20x_led_attrs[] = {
>>> +	&dev_attr_control.attr,
>>> +	&dev_attr_mode.attr,
>>> +	NULL,
>>> +};
>>> +ATTRIBUTE_GROUPS(axp20x_led);
>> I can't really say whether adding sysfs handles for this is the right
>> thing to do, but if it is you should document the interface.
> It is not. See "Add Intel Cherry Trail Whiskey Cove PMIC LEDs" thread
> in the last few days.

What I understood is that there will be changes in the LED core, exporting
a new sysfs entry "hw_control". Then I should set HW_CONTROL flag for the
device and add hw_control_get/set callback functions. And this can happen
when the LED core is updated. Right?

However I didn't understand how (in my case) hardware controlled pattern
will be selected?

Best regards,
Stefan Mavrodiev

>>> +	if (!of_property_read_u8(np, "x-powers,charger-mode", &value)) {
>>> +		priv->ctrl = AXP20X_CHGLED_CTRL_CHARGER;
>>> +		priv->mode = (value < 2) ? value : 0;
>>> +	} else {
>>> +		priv->ctrl = AXP20X_CHGLED_CTRL_MANUAL;
>>> +	}
>> I'm not sure we want to make this a property of the device
>> tree. Changing the device tree isn't an option for some users, so we
>> need to make sure we can change it even if we can't change the device
>> tree.
> We want this to be configurable at run time. It can get default from
> the device tree.
>
> If we go for the "hardware" trigger, you'll get it for free.
>
> Best regards,
> 								Pavel

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

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

* Re: [PATCH v2 5/8] arm64: dts: allwinner: Enable AXP803 CHGLED for Olimex boards
  2019-02-15 18:49     ` Pavel Machek
  (?)
@ 2019-02-19  7:43       ` Stefan Mavrodiev
  -1 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-19  7:43 UTC (permalink / raw)
  To: Pavel Machek, Stefan Mavrodiev
  Cc: Jacek Anaszewski, Rob Herring, Mark Rutland, Chen-Yu Tsai,
	Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support


On 2/15/19 8:49 PM, Pavel Machek wrote:
> On Fri 2019-02-15 13:50:10, Stefan Mavrodiev wrote:
>> Teres-I and A64-OLinuXino commes with populated LED connected
>> to AXP803. So to be used as battery indication, pass the LED control
>> to the charger itself in mode 0 ( FULL while charging, OFF no battery
>> or completed).
>>
>> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
>> ---
>>   arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts | 6 ++++++
>>   arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts   | 6 ++++++
>>   2 files changed, 12 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
>> index f7a4bccaa5d4..7d6930319fba 100644
>> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
>> @@ -177,6 +177,12 @@
>>   
>>   #include "axp803.dtsi"
>>   
>> +&axp_led {
>> +	label = "a64-olinuxino:yellow:chgled";
> There's little chance userspace will recognize this led.
>
> Can you make it "platform:yellow:charging" and fix it for other
> boards, too?

Sure.

Best regards,
Stefan Mavrodiev

>
> 									Pavel

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

* Re: [PATCH v2 5/8] arm64: dts: allwinner: Enable AXP803 CHGLED for Olimex boards
@ 2019-02-19  7:43       ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-19  7:43 UTC (permalink / raw)
  To: Pavel Machek, Stefan Mavrodiev
  Cc: Jacek Anaszewski, Rob Herring, Mark Rutland, Chen-Yu Tsai,
	Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support


On 2/15/19 8:49 PM, Pavel Machek wrote:
> On Fri 2019-02-15 13:50:10, Stefan Mavrodiev wrote:
>> Teres-I and A64-OLinuXino commes with populated LED connected
>> to AXP803. So to be used as battery indication, pass the LED control
>> to the charger itself in mode 0 ( FULL while charging, OFF no battery
>> or completed).
>>
>> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
>> ---
>>   arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts | 6 ++++++
>>   arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts   | 6 ++++++
>>   2 files changed, 12 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
>> index f7a4bccaa5d4..7d6930319fba 100644
>> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
>> @@ -177,6 +177,12 @@
>>   
>>   #include "axp803.dtsi"
>>   
>> +&axp_led {
>> +	label = "a64-olinuxino:yellow:chgled";
> There's little chance userspace will recognize this led.
>
> Can you make it "platform:yellow:charging" and fix it for other
> boards, too?

Sure.

Best regards,
Stefan Mavrodiev

>
> 									Pavel

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

* Re: [PATCH v2 5/8] arm64: dts: allwinner: Enable AXP803 CHGLED for Olimex boards
@ 2019-02-19  7:43       ` Stefan Mavrodiev
  0 siblings, 0 replies; 54+ messages in thread
From: Stefan Mavrodiev @ 2019-02-19  7:43 UTC (permalink / raw)
  To: Pavel Machek, Stefan Mavrodiev
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Maxime Ripard,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	Chen-Yu Tsai, Rob Herring, Jacek Anaszewski, Lee Jones,
	open list:LED SUBSYSTEM,
	moderated list:ARM/Allwinner sunXi SoC support


On 2/15/19 8:49 PM, Pavel Machek wrote:
> On Fri 2019-02-15 13:50:10, Stefan Mavrodiev wrote:
>> Teres-I and A64-OLinuXino commes with populated LED connected
>> to AXP803. So to be used as battery indication, pass the LED control
>> to the charger itself in mode 0 ( FULL while charging, OFF no battery
>> or completed).
>>
>> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
>> ---
>>   arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts | 6 ++++++
>>   arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts   | 6 ++++++
>>   2 files changed, 12 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
>> index f7a4bccaa5d4..7d6930319fba 100644
>> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-olinuxino.dts
>> @@ -177,6 +177,12 @@
>>   
>>   #include "axp803.dtsi"
>>   
>> +&axp_led {
>> +	label = "a64-olinuxino:yellow:chgled";
> There's little chance userspace will recognize this led.
>
> Can you make it "platform:yellow:charging" and fix it for other
> boards, too?

Sure.

Best regards,
Stefan Mavrodiev

>
> 									Pavel

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

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

* Re: [PATCH v2 3/8] dt-bindings: leds: Add binding for axp20x-led device driver
  2019-02-15 11:50   ` Stefan Mavrodiev
  (?)
@ 2019-02-22 19:51     ` Rob Herring
  -1 siblings, 0 replies; 54+ messages in thread
From: Rob Herring @ 2019-02-22 19:51 UTC (permalink / raw)
  To: Stefan Mavrodiev
  Cc: Jacek Anaszewski, Pavel Machek, Mark Rutland, Chen-Yu Tsai,
	Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support

On Fri, Feb 15, 2019 at 01:50:08PM +0200, Stefan Mavrodiev wrote:
> This adds the devicetree bindings for charge led indicator found
> on most of X-Powers AXP20X PMICs.
> 
> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
> ---
>  .../devicetree/bindings/leds/leds-axp20x.txt  | 74 +++++++++++++++++++
>  1 file changed, 74 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-axp20x.txt
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-axp20x.txt b/Documentation/devicetree/bindings/leds/leds-axp20x.txt
> new file mode 100644
> index 000000000000..5a83ad06796d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-axp20x.txt
> @@ -0,0 +1,74 @@
> +Device Tree Bindings for LED support on X-Powers PMIC
> +
> +Most of the X-Powers PMICs have integrated battery charger with LED indicator.
> +The output is open-drain, so the state is either high-Z or output-low. The
> +driver is a subnode of AXP20X MFD driver, since it uses shared bus with all
> +other cells.
> +The LED can be controlled either manually or automatically. Then in automatic
> +(controlled by the charger) there are two indication modes:
> +
> +Mode-A
> +======
> +- output-low:		Charging
> +- high-Z		Not charging
> +- 1Hz flashing:		Abnormal alarm
> +- 4Hz flashing		Overvoltage alarm
> +
> +Mode-B
> +======
> +- output-low:		Battery full
> +- high-Z		Not charging
> +- 1Hz flashing:		Charging
> +- 4Hz flashing		Overvoltage or abnormal alarm
> +
> +The control and the mode can be changed from sysfs.
> +
> +For AXP20X MFD bindings see:
> +Documentation/devicetree/bindings/mfd/axp20x.txt
> +
> +Required properties:
> +- compatible : Must be "x-powers,axp20x-led"
> +
> +Supported common LED properties, see ./common.txt for more informationn
> +- label : See Documentation/devicetree/bindings/leds/common.txt
> +- linux,default-trigger : See Documentation/devicetree/bindings/leds/common.txt
> +- default-state: See Documentation/devicetree/bindings/leds/common.txt
> +
> +Optional properties:
> +- x-powers,charger-mode: 0 for Mode-A, 1 for Mode-B
> +			 If omitted, then the control is set to manual mode.
> +			 On invalid value, Mode-A is used.
> +
> +
> +Example:
> +
> +	axp803: pmic@3a3 {
> +		compatible = "x-powers,axp803";
> +
> +		...
> +
> +		led@0 {

What's the 0 for? A unit address without a reg property is not valid.

> +			compatible = "x-powers,axp20x-led";
> +			status = "okay";
> +
> +			label = "axp20x:yellow:chgled";
> +			linux,default-trigger = "timer";
> +			default-state = "on";
> +		};
> +	};
> +
> +or
> +
> +	axp803: pmic@3a3 {
> +		compatible = "x-powers,axp803";
> +
> +		...
> +
> +		led@0 {
> +			compatible = "x-powers,axp20x-led";
> +			status = "okay";
> +
> +			label = "axp20x:yellow:chgled";
> +			x-powers,charger-mode = <1>;
> +		};
> +	};
> -- 
> 2.17.1
> 

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

* Re: [PATCH v2 3/8] dt-bindings: leds: Add binding for axp20x-led device driver
@ 2019-02-22 19:51     ` Rob Herring
  0 siblings, 0 replies; 54+ messages in thread
From: Rob Herring @ 2019-02-22 19:51 UTC (permalink / raw)
  To: Stefan Mavrodiev
  Cc: Jacek Anaszewski, Pavel Machek, Mark Rutland, Chen-Yu Tsai,
	Maxime Ripard, Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support

On Fri, Feb 15, 2019 at 01:50:08PM +0200, Stefan Mavrodiev wrote:
> This adds the devicetree bindings for charge led indicator found
> on most of X-Powers AXP20X PMICs.
> 
> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
> ---
>  .../devicetree/bindings/leds/leds-axp20x.txt  | 74 +++++++++++++++++++
>  1 file changed, 74 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-axp20x.txt
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-axp20x.txt b/Documentation/devicetree/bindings/leds/leds-axp20x.txt
> new file mode 100644
> index 000000000000..5a83ad06796d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-axp20x.txt
> @@ -0,0 +1,74 @@
> +Device Tree Bindings for LED support on X-Powers PMIC
> +
> +Most of the X-Powers PMICs have integrated battery charger with LED indicator.
> +The output is open-drain, so the state is either high-Z or output-low. The
> +driver is a subnode of AXP20X MFD driver, since it uses shared bus with all
> +other cells.
> +The LED can be controlled either manually or automatically. Then in automatic
> +(controlled by the charger) there are two indication modes:
> +
> +Mode-A
> +======
> +- output-low:		Charging
> +- high-Z		Not charging
> +- 1Hz flashing:		Abnormal alarm
> +- 4Hz flashing		Overvoltage alarm
> +
> +Mode-B
> +======
> +- output-low:		Battery full
> +- high-Z		Not charging
> +- 1Hz flashing:		Charging
> +- 4Hz flashing		Overvoltage or abnormal alarm
> +
> +The control and the mode can be changed from sysfs.
> +
> +For AXP20X MFD bindings see:
> +Documentation/devicetree/bindings/mfd/axp20x.txt
> +
> +Required properties:
> +- compatible : Must be "x-powers,axp20x-led"
> +
> +Supported common LED properties, see ./common.txt for more informationn
> +- label : See Documentation/devicetree/bindings/leds/common.txt
> +- linux,default-trigger : See Documentation/devicetree/bindings/leds/common.txt
> +- default-state: See Documentation/devicetree/bindings/leds/common.txt
> +
> +Optional properties:
> +- x-powers,charger-mode: 0 for Mode-A, 1 for Mode-B
> +			 If omitted, then the control is set to manual mode.
> +			 On invalid value, Mode-A is used.
> +
> +
> +Example:
> +
> +	axp803: pmic@3a3 {
> +		compatible = "x-powers,axp803";
> +
> +		...
> +
> +		led@0 {

What's the 0 for? A unit address without a reg property is not valid.

> +			compatible = "x-powers,axp20x-led";
> +			status = "okay";
> +
> +			label = "axp20x:yellow:chgled";
> +			linux,default-trigger = "timer";
> +			default-state = "on";
> +		};
> +	};
> +
> +or
> +
> +	axp803: pmic@3a3 {
> +		compatible = "x-powers,axp803";
> +
> +		...
> +
> +		led@0 {
> +			compatible = "x-powers,axp20x-led";
> +			status = "okay";
> +
> +			label = "axp20x:yellow:chgled";
> +			x-powers,charger-mode = <1>;
> +		};
> +	};
> -- 
> 2.17.1
> 

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

* Re: [PATCH v2 3/8] dt-bindings: leds: Add binding for axp20x-led device driver
@ 2019-02-22 19:51     ` Rob Herring
  0 siblings, 0 replies; 54+ messages in thread
From: Rob Herring @ 2019-02-22 19:51 UTC (permalink / raw)
  To: Stefan Mavrodiev
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Maxime Ripard,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	Chen-Yu Tsai, Jacek Anaszewski, Pavel Machek, Lee Jones,
	open list:LED SUBSYSTEM,
	moderated list:ARM/Allwinner sunXi SoC support

On Fri, Feb 15, 2019 at 01:50:08PM +0200, Stefan Mavrodiev wrote:
> This adds the devicetree bindings for charge led indicator found
> on most of X-Powers AXP20X PMICs.
> 
> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
> ---
>  .../devicetree/bindings/leds/leds-axp20x.txt  | 74 +++++++++++++++++++
>  1 file changed, 74 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-axp20x.txt
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-axp20x.txt b/Documentation/devicetree/bindings/leds/leds-axp20x.txt
> new file mode 100644
> index 000000000000..5a83ad06796d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-axp20x.txt
> @@ -0,0 +1,74 @@
> +Device Tree Bindings for LED support on X-Powers PMIC
> +
> +Most of the X-Powers PMICs have integrated battery charger with LED indicator.
> +The output is open-drain, so the state is either high-Z or output-low. The
> +driver is a subnode of AXP20X MFD driver, since it uses shared bus with all
> +other cells.
> +The LED can be controlled either manually or automatically. Then in automatic
> +(controlled by the charger) there are two indication modes:
> +
> +Mode-A
> +======
> +- output-low:		Charging
> +- high-Z		Not charging
> +- 1Hz flashing:		Abnormal alarm
> +- 4Hz flashing		Overvoltage alarm
> +
> +Mode-B
> +======
> +- output-low:		Battery full
> +- high-Z		Not charging
> +- 1Hz flashing:		Charging
> +- 4Hz flashing		Overvoltage or abnormal alarm
> +
> +The control and the mode can be changed from sysfs.
> +
> +For AXP20X MFD bindings see:
> +Documentation/devicetree/bindings/mfd/axp20x.txt
> +
> +Required properties:
> +- compatible : Must be "x-powers,axp20x-led"
> +
> +Supported common LED properties, see ./common.txt for more informationn
> +- label : See Documentation/devicetree/bindings/leds/common.txt
> +- linux,default-trigger : See Documentation/devicetree/bindings/leds/common.txt
> +- default-state: See Documentation/devicetree/bindings/leds/common.txt
> +
> +Optional properties:
> +- x-powers,charger-mode: 0 for Mode-A, 1 for Mode-B
> +			 If omitted, then the control is set to manual mode.
> +			 On invalid value, Mode-A is used.
> +
> +
> +Example:
> +
> +	axp803: pmic@3a3 {
> +		compatible = "x-powers,axp803";
> +
> +		...
> +
> +		led@0 {

What's the 0 for? A unit address without a reg property is not valid.

> +			compatible = "x-powers,axp20x-led";
> +			status = "okay";
> +
> +			label = "axp20x:yellow:chgled";
> +			linux,default-trigger = "timer";
> +			default-state = "on";
> +		};
> +	};
> +
> +or
> +
> +	axp803: pmic@3a3 {
> +		compatible = "x-powers,axp803";
> +
> +		...
> +
> +		led@0 {
> +			compatible = "x-powers,axp20x-led";
> +			status = "okay";
> +
> +			label = "axp20x:yellow:chgled";
> +			x-powers,charger-mode = <1>;
> +		};
> +	};
> -- 
> 2.17.1
> 

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

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

* Re: [PATCH v2 3/8] dt-bindings: leds: Add binding for axp20x-led device driver
  2019-02-22 19:51     ` Rob Herring
  (?)
@ 2019-02-23 13:02       ` Jacek Anaszewski
  -1 siblings, 0 replies; 54+ messages in thread
From: Jacek Anaszewski @ 2019-02-23 13:02 UTC (permalink / raw)
  To: Rob Herring, Stefan Mavrodiev
  Cc: Pavel Machek, Mark Rutland, Chen-Yu Tsai, Maxime Ripard,
	Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support

On 2/22/19 8:51 PM, Rob Herring wrote:
> On Fri, Feb 15, 2019 at 01:50:08PM +0200, Stefan Mavrodiev wrote:
>> This adds the devicetree bindings for charge led indicator found
>> on most of X-Powers AXP20X PMICs.
>>
>> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
>> ---
>>   .../devicetree/bindings/leds/leds-axp20x.txt  | 74 +++++++++++++++++++
>>   1 file changed, 74 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/leds/leds-axp20x.txt
>>
>> diff --git a/Documentation/devicetree/bindings/leds/leds-axp20x.txt b/Documentation/devicetree/bindings/leds/leds-axp20x.txt
>> new file mode 100644
>> index 000000000000..5a83ad06796d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/leds/leds-axp20x.txt
>> @@ -0,0 +1,74 @@
>> +Device Tree Bindings for LED support on X-Powers PMIC
>> +
>> +Most of the X-Powers PMICs have integrated battery charger with LED indicator.
>> +The output is open-drain, so the state is either high-Z or output-low. The
>> +driver is a subnode of AXP20X MFD driver, since it uses shared bus with all
>> +other cells.
>> +The LED can be controlled either manually or automatically. Then in automatic
>> +(controlled by the charger) there are two indication modes:
>> +
>> +Mode-A
>> +======
>> +- output-low:		Charging
>> +- high-Z		Not charging
>> +- 1Hz flashing:		Abnormal alarm
>> +- 4Hz flashing		Overvoltage alarm
>> +
>> +Mode-B
>> +======
>> +- output-low:		Battery full
>> +- high-Z		Not charging
>> +- 1Hz flashing:		Charging
>> +- 4Hz flashing		Overvoltage or abnormal alarm
>> +
>> +The control and the mode can be changed from sysfs.
>> +
>> +For AXP20X MFD bindings see:
>> +Documentation/devicetree/bindings/mfd/axp20x.txt
>> +
>> +Required properties:
>> +- compatible : Must be "x-powers,axp20x-led"
>> +
>> +Supported common LED properties, see ./common.txt for more informationn
>> +- label : See Documentation/devicetree/bindings/leds/common.txt
>> +- linux,default-trigger : See Documentation/devicetree/bindings/leds/common.txt
>> +- default-state: See Documentation/devicetree/bindings/leds/common.txt
>> +
>> +Optional properties:
>> +- x-powers,charger-mode: 0 for Mode-A, 1 for Mode-B
>> +			 If omitted, then the control is set to manual mode.
>> +			 On invalid value, Mode-A is used.
>> +
>> +
>> +Example:
>> +
>> +	axp803: pmic@3a3 {
>> +		compatible = "x-powers,axp803";
>> +
>> +		...
>> +
>> +		led@0 {
> 
> What's the 0 for? A unit address without a reg property is not valid.

It was done on my request, but now I see it was an omission.

Since this node describes a LED controller, then it should
be called "led-controller".

>> +			compatible = "x-powers,axp20x-led";
>> +			status = "okay";

And below part should be in a child node.

Stefan, please compare
Documentation/devicetree/bindings/leds/common.txt and
other LED bindings.

>> +			label = "axp20x:yellow:chgled";

Moreover "axp20x:" part should be removed from here added in the driver.

Please refer to drivers/leds/leds-cr0014114.c on how we
do that now.

>> +			linux,default-trigger = "timer";
>> +			default-state = "on";
>> +		};
>> +	};
>> +
>> +or
>> +
>> +	axp803: pmic@3a3 {
>> +		compatible = "x-powers,axp803";
>> +
>> +		...
>> +
>> +		led@0 {
>> +			compatible = "x-powers,axp20x-led";
>> +			status = "okay";
>> +
>> +			label = "axp20x:yellow:chgled";
>> +			x-powers,charger-mode = <1>;
>> +		};
>> +	};
>> -- 
>> 2.17.1
>>
> 

-- 
Best regards,
Jacek Anaszewski

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

* Re: [PATCH v2 3/8] dt-bindings: leds: Add binding for axp20x-led device driver
@ 2019-02-23 13:02       ` Jacek Anaszewski
  0 siblings, 0 replies; 54+ messages in thread
From: Jacek Anaszewski @ 2019-02-23 13:02 UTC (permalink / raw)
  To: Rob Herring, Stefan Mavrodiev
  Cc: Pavel Machek, Mark Rutland, Chen-Yu Tsai, Maxime Ripard,
	Lee Jones, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support

On 2/22/19 8:51 PM, Rob Herring wrote:
> On Fri, Feb 15, 2019 at 01:50:08PM +0200, Stefan Mavrodiev wrote:
>> This adds the devicetree bindings for charge led indicator found
>> on most of X-Powers AXP20X PMICs.
>>
>> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
>> ---
>>   .../devicetree/bindings/leds/leds-axp20x.txt  | 74 +++++++++++++++++++
>>   1 file changed, 74 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/leds/leds-axp20x.txt
>>
>> diff --git a/Documentation/devicetree/bindings/leds/leds-axp20x.txt b/Documentation/devicetree/bindings/leds/leds-axp20x.txt
>> new file mode 100644
>> index 000000000000..5a83ad06796d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/leds/leds-axp20x.txt
>> @@ -0,0 +1,74 @@
>> +Device Tree Bindings for LED support on X-Powers PMIC
>> +
>> +Most of the X-Powers PMICs have integrated battery charger with LED indicator.
>> +The output is open-drain, so the state is either high-Z or output-low. The
>> +driver is a subnode of AXP20X MFD driver, since it uses shared bus with all
>> +other cells.
>> +The LED can be controlled either manually or automatically. Then in automatic
>> +(controlled by the charger) there are two indication modes:
>> +
>> +Mode-A
>> +======
>> +- output-low:		Charging
>> +- high-Z		Not charging
>> +- 1Hz flashing:		Abnormal alarm
>> +- 4Hz flashing		Overvoltage alarm
>> +
>> +Mode-B
>> +======
>> +- output-low:		Battery full
>> +- high-Z		Not charging
>> +- 1Hz flashing:		Charging
>> +- 4Hz flashing		Overvoltage or abnormal alarm
>> +
>> +The control and the mode can be changed from sysfs.
>> +
>> +For AXP20X MFD bindings see:
>> +Documentation/devicetree/bindings/mfd/axp20x.txt
>> +
>> +Required properties:
>> +- compatible : Must be "x-powers,axp20x-led"
>> +
>> +Supported common LED properties, see ./common.txt for more informationn
>> +- label : See Documentation/devicetree/bindings/leds/common.txt
>> +- linux,default-trigger : See Documentation/devicetree/bindings/leds/common.txt
>> +- default-state: See Documentation/devicetree/bindings/leds/common.txt
>> +
>> +Optional properties:
>> +- x-powers,charger-mode: 0 for Mode-A, 1 for Mode-B
>> +			 If omitted, then the control is set to manual mode.
>> +			 On invalid value, Mode-A is used.
>> +
>> +
>> +Example:
>> +
>> +	axp803: pmic@3a3 {
>> +		compatible = "x-powers,axp803";
>> +
>> +		...
>> +
>> +		led@0 {
> 
> What's the 0 for? A unit address without a reg property is not valid.

It was done on my request, but now I see it was an omission.

Since this node describes a LED controller, then it should
be called "led-controller".

>> +			compatible = "x-powers,axp20x-led";
>> +			status = "okay";

And below part should be in a child node.

Stefan, please compare
Documentation/devicetree/bindings/leds/common.txt and
other LED bindings.

>> +			label = "axp20x:yellow:chgled";

Moreover "axp20x:" part should be removed from here added in the driver.

Please refer to drivers/leds/leds-cr0014114.c on how we
do that now.

>> +			linux,default-trigger = "timer";
>> +			default-state = "on";
>> +		};
>> +	};
>> +
>> +or
>> +
>> +	axp803: pmic@3a3 {
>> +		compatible = "x-powers,axp803";
>> +
>> +		...
>> +
>> +		led@0 {
>> +			compatible = "x-powers,axp20x-led";
>> +			status = "okay";
>> +
>> +			label = "axp20x:yellow:chgled";
>> +			x-powers,charger-mode = <1>;
>> +		};
>> +	};
>> -- 
>> 2.17.1
>>
> 

-- 
Best regards,
Jacek Anaszewski

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

* Re: [PATCH v2 3/8] dt-bindings: leds: Add binding for axp20x-led device driver
@ 2019-02-23 13:02       ` Jacek Anaszewski
  0 siblings, 0 replies; 54+ messages in thread
From: Jacek Anaszewski @ 2019-02-23 13:02 UTC (permalink / raw)
  To: Rob Herring, Stefan Mavrodiev
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Maxime Ripard,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	Chen-Yu Tsai, moderated list:ARM/Allwinner sunXi SoC support,
	Pavel Machek, Lee Jones, open list:LED SUBSYSTEM

On 2/22/19 8:51 PM, Rob Herring wrote:
> On Fri, Feb 15, 2019 at 01:50:08PM +0200, Stefan Mavrodiev wrote:
>> This adds the devicetree bindings for charge led indicator found
>> on most of X-Powers AXP20X PMICs.
>>
>> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
>> ---
>>   .../devicetree/bindings/leds/leds-axp20x.txt  | 74 +++++++++++++++++++
>>   1 file changed, 74 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/leds/leds-axp20x.txt
>>
>> diff --git a/Documentation/devicetree/bindings/leds/leds-axp20x.txt b/Documentation/devicetree/bindings/leds/leds-axp20x.txt
>> new file mode 100644
>> index 000000000000..5a83ad06796d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/leds/leds-axp20x.txt
>> @@ -0,0 +1,74 @@
>> +Device Tree Bindings for LED support on X-Powers PMIC
>> +
>> +Most of the X-Powers PMICs have integrated battery charger with LED indicator.
>> +The output is open-drain, so the state is either high-Z or output-low. The
>> +driver is a subnode of AXP20X MFD driver, since it uses shared bus with all
>> +other cells.
>> +The LED can be controlled either manually or automatically. Then in automatic
>> +(controlled by the charger) there are two indication modes:
>> +
>> +Mode-A
>> +======
>> +- output-low:		Charging
>> +- high-Z		Not charging
>> +- 1Hz flashing:		Abnormal alarm
>> +- 4Hz flashing		Overvoltage alarm
>> +
>> +Mode-B
>> +======
>> +- output-low:		Battery full
>> +- high-Z		Not charging
>> +- 1Hz flashing:		Charging
>> +- 4Hz flashing		Overvoltage or abnormal alarm
>> +
>> +The control and the mode can be changed from sysfs.
>> +
>> +For AXP20X MFD bindings see:
>> +Documentation/devicetree/bindings/mfd/axp20x.txt
>> +
>> +Required properties:
>> +- compatible : Must be "x-powers,axp20x-led"
>> +
>> +Supported common LED properties, see ./common.txt for more informationn
>> +- label : See Documentation/devicetree/bindings/leds/common.txt
>> +- linux,default-trigger : See Documentation/devicetree/bindings/leds/common.txt
>> +- default-state: See Documentation/devicetree/bindings/leds/common.txt
>> +
>> +Optional properties:
>> +- x-powers,charger-mode: 0 for Mode-A, 1 for Mode-B
>> +			 If omitted, then the control is set to manual mode.
>> +			 On invalid value, Mode-A is used.
>> +
>> +
>> +Example:
>> +
>> +	axp803: pmic@3a3 {
>> +		compatible = "x-powers,axp803";
>> +
>> +		...
>> +
>> +		led@0 {
> 
> What's the 0 for? A unit address without a reg property is not valid.

It was done on my request, but now I see it was an omission.

Since this node describes a LED controller, then it should
be called "led-controller".

>> +			compatible = "x-powers,axp20x-led";
>> +			status = "okay";

And below part should be in a child node.

Stefan, please compare
Documentation/devicetree/bindings/leds/common.txt and
other LED bindings.

>> +			label = "axp20x:yellow:chgled";

Moreover "axp20x:" part should be removed from here added in the driver.

Please refer to drivers/leds/leds-cr0014114.c on how we
do that now.

>> +			linux,default-trigger = "timer";
>> +			default-state = "on";
>> +		};
>> +	};
>> +
>> +or
>> +
>> +	axp803: pmic@3a3 {
>> +		compatible = "x-powers,axp803";
>> +
>> +		...
>> +
>> +		led@0 {
>> +			compatible = "x-powers,axp20x-led";
>> +			status = "okay";
>> +
>> +			label = "axp20x:yellow:chgled";
>> +			x-powers,charger-mode = <1>;
>> +		};
>> +	};
>> -- 
>> 2.17.1
>>
> 

-- 
Best regards,
Jacek Anaszewski

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

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

* Re: [PATCH v2 2/8] mfd: axp20x: Add axp20x-led cell
  2019-02-15 11:50   ` Stefan Mavrodiev
  (?)
@ 2019-03-25 10:01     ` Lee Jones
  -1 siblings, 0 replies; 54+ messages in thread
From: Lee Jones @ 2019-03-25 10:01 UTC (permalink / raw)
  To: Stefan Mavrodiev
  Cc: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support

On Fri, 15 Feb 2019, Stefan Mavrodiev wrote:

> Add axp20x-led cell for AXP20x, AXP221, AXP223, AXP228, AXP803, AXP809
> and AXP813.
> 
> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
> ---
>  drivers/mfd/axp20x.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 3c97f2c0fdfe..e6ab078f0462 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -610,6 +610,9 @@ static const struct mfd_cell axp20x_cells[] = {
>  		.of_compatible	= "x-powers,axp202-usb-power-supply",
>  		.num_resources	= ARRAY_SIZE(axp20x_usb_power_supply_resources),
>  		.resources	= axp20x_usb_power_supply_resources,
> +	}, {
> +		.name		= "axp20x-led",
> +		.of_compatible	= "x-powers,axp20x-led",
>  	},
>  };
>  
> @@ -636,6 +639,9 @@ static const struct mfd_cell axp221_cells[] = {
>  		.of_compatible	= "x-powers,axp221-usb-power-supply",
>  		.num_resources	= ARRAY_SIZE(axp22x_usb_power_supply_resources),
>  		.resources	= axp22x_usb_power_supply_resources,
> +	}, {
> +		.name		= "axp20x-led",
> +		.of_compatible	= "x-powers,axp20x-led",
>  	},
>  };
>  
> @@ -662,6 +668,9 @@ static const struct mfd_cell axp223_cells[] = {
>  		.of_compatible	= "x-powers,axp223-usb-power-supply",
>  		.num_resources	= ARRAY_SIZE(axp22x_usb_power_supply_resources),
>  		.resources	= axp22x_usb_power_supply_resources,
> +	}, {
> +		.name		= "axp20x-led",
> +		.of_compatible	= "x-powers,axp20x-led",
>  	},
>  };
>  
> @@ -719,6 +728,9 @@ static const struct mfd_cell axp288_cells[] = {
>  		.resources	= axp288_power_button_resources,
>  	}, {
>  		.name		= "axp288_pmic_acpi",
> +	}, {
> +		.name		= "axp20x-led",
> +		.of_compatible	= "x-powers,axp20x-led",
>  	},
>  };
>  
> @@ -741,8 +753,12 @@ static const struct mfd_cell axp803_cells[] = {
>  		.of_compatible	= "x-powers,axp813-ac-power-supply",
>  		.num_resources	= ARRAY_SIZE(axp20x_ac_power_supply_resources),
>  		.resources	= axp20x_ac_power_supply_resources,
> +	}, {
> +		.name		= "axp20x-regulator"
> +	}, {
> +		.name		= "axp20x-led",
> +		.of_compatible	= "x-powers,axp20x-led",
>  	},
> -	{	.name		= "axp20x-regulator" },

Nit: Please keep this at the bottom and as a one line entry.

Once fixed, please submit with my:

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

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

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

* Re: [PATCH v2 2/8] mfd: axp20x: Add axp20x-led cell
@ 2019-03-25 10:01     ` Lee Jones
  0 siblings, 0 replies; 54+ messages in thread
From: Lee Jones @ 2019-03-25 10:01 UTC (permalink / raw)
  To: Stefan Mavrodiev
  Cc: Jacek Anaszewski, Pavel Machek, Rob Herring, Mark Rutland,
	Chen-Yu Tsai, Maxime Ripard, open list:LED SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	moderated list:ARM/Allwinner sunXi SoC support

On Fri, 15 Feb 2019, Stefan Mavrodiev wrote:

> Add axp20x-led cell for AXP20x, AXP221, AXP223, AXP228, AXP803, AXP809
> and AXP813.
> 
> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
> ---
>  drivers/mfd/axp20x.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 3c97f2c0fdfe..e6ab078f0462 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -610,6 +610,9 @@ static const struct mfd_cell axp20x_cells[] = {
>  		.of_compatible	= "x-powers,axp202-usb-power-supply",
>  		.num_resources	= ARRAY_SIZE(axp20x_usb_power_supply_resources),
>  		.resources	= axp20x_usb_power_supply_resources,
> +	}, {
> +		.name		= "axp20x-led",
> +		.of_compatible	= "x-powers,axp20x-led",
>  	},
>  };
>  
> @@ -636,6 +639,9 @@ static const struct mfd_cell axp221_cells[] = {
>  		.of_compatible	= "x-powers,axp221-usb-power-supply",
>  		.num_resources	= ARRAY_SIZE(axp22x_usb_power_supply_resources),
>  		.resources	= axp22x_usb_power_supply_resources,
> +	}, {
> +		.name		= "axp20x-led",
> +		.of_compatible	= "x-powers,axp20x-led",
>  	},
>  };
>  
> @@ -662,6 +668,9 @@ static const struct mfd_cell axp223_cells[] = {
>  		.of_compatible	= "x-powers,axp223-usb-power-supply",
>  		.num_resources	= ARRAY_SIZE(axp22x_usb_power_supply_resources),
>  		.resources	= axp22x_usb_power_supply_resources,
> +	}, {
> +		.name		= "axp20x-led",
> +		.of_compatible	= "x-powers,axp20x-led",
>  	},
>  };
>  
> @@ -719,6 +728,9 @@ static const struct mfd_cell axp288_cells[] = {
>  		.resources	= axp288_power_button_resources,
>  	}, {
>  		.name		= "axp288_pmic_acpi",
> +	}, {
> +		.name		= "axp20x-led",
> +		.of_compatible	= "x-powers,axp20x-led",
>  	},
>  };
>  
> @@ -741,8 +753,12 @@ static const struct mfd_cell axp803_cells[] = {
>  		.of_compatible	= "x-powers,axp813-ac-power-supply",
>  		.num_resources	= ARRAY_SIZE(axp20x_ac_power_supply_resources),
>  		.resources	= axp20x_ac_power_supply_resources,
> +	}, {
> +		.name		= "axp20x-regulator"
> +	}, {
> +		.name		= "axp20x-led",
> +		.of_compatible	= "x-powers,axp20x-led",
>  	},
> -	{	.name		= "axp20x-regulator" },

Nit: Please keep this at the bottom and as a one line entry.

Once fixed, please submit with my:

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

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

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

* Re: [PATCH v2 2/8] mfd: axp20x: Add axp20x-led cell
@ 2019-03-25 10:01     ` Lee Jones
  0 siblings, 0 replies; 54+ messages in thread
From: Lee Jones @ 2019-03-25 10:01 UTC (permalink / raw)
  To: Stefan Mavrodiev
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Maxime Ripard,
	open list:X-POWERS MULTIFUNCTION PMIC DEVICE DRIVERS,
	Chen-Yu Tsai, Rob Herring, Jacek Anaszewski, Pavel Machek,
	open list:LED SUBSYSTEM,
	moderated list:ARM/Allwinner sunXi SoC support

On Fri, 15 Feb 2019, Stefan Mavrodiev wrote:

> Add axp20x-led cell for AXP20x, AXP221, AXP223, AXP228, AXP803, AXP809
> and AXP813.
> 
> Signed-off-by: Stefan Mavrodiev <stefan@olimex.com>
> ---
>  drivers/mfd/axp20x.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 3c97f2c0fdfe..e6ab078f0462 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -610,6 +610,9 @@ static const struct mfd_cell axp20x_cells[] = {
>  		.of_compatible	= "x-powers,axp202-usb-power-supply",
>  		.num_resources	= ARRAY_SIZE(axp20x_usb_power_supply_resources),
>  		.resources	= axp20x_usb_power_supply_resources,
> +	}, {
> +		.name		= "axp20x-led",
> +		.of_compatible	= "x-powers,axp20x-led",
>  	},
>  };
>  
> @@ -636,6 +639,9 @@ static const struct mfd_cell axp221_cells[] = {
>  		.of_compatible	= "x-powers,axp221-usb-power-supply",
>  		.num_resources	= ARRAY_SIZE(axp22x_usb_power_supply_resources),
>  		.resources	= axp22x_usb_power_supply_resources,
> +	}, {
> +		.name		= "axp20x-led",
> +		.of_compatible	= "x-powers,axp20x-led",
>  	},
>  };
>  
> @@ -662,6 +668,9 @@ static const struct mfd_cell axp223_cells[] = {
>  		.of_compatible	= "x-powers,axp223-usb-power-supply",
>  		.num_resources	= ARRAY_SIZE(axp22x_usb_power_supply_resources),
>  		.resources	= axp22x_usb_power_supply_resources,
> +	}, {
> +		.name		= "axp20x-led",
> +		.of_compatible	= "x-powers,axp20x-led",
>  	},
>  };
>  
> @@ -719,6 +728,9 @@ static const struct mfd_cell axp288_cells[] = {
>  		.resources	= axp288_power_button_resources,
>  	}, {
>  		.name		= "axp288_pmic_acpi",
> +	}, {
> +		.name		= "axp20x-led",
> +		.of_compatible	= "x-powers,axp20x-led",
>  	},
>  };
>  
> @@ -741,8 +753,12 @@ static const struct mfd_cell axp803_cells[] = {
>  		.of_compatible	= "x-powers,axp813-ac-power-supply",
>  		.num_resources	= ARRAY_SIZE(axp20x_ac_power_supply_resources),
>  		.resources	= axp20x_ac_power_supply_resources,
> +	}, {
> +		.name		= "axp20x-regulator"
> +	}, {
> +		.name		= "axp20x-led",
> +		.of_compatible	= "x-powers,axp20x-led",
>  	},
> -	{	.name		= "axp20x-regulator" },

Nit: Please keep this at the bottom and as a one line entry.

Once fixed, please submit with my:

For my own reference:
  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

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

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

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

end of thread, other threads:[~2019-03-25 10:02 UTC | newest]

Thread overview: 54+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-15 11:50 [PATCH v2 0/8] leds: Add AXP20X CHGLED Stefan Mavrodiev
2019-02-15 11:50 ` Stefan Mavrodiev
2019-02-15 11:50 ` Stefan Mavrodiev
2019-02-15 11:50 ` [PATCH v2 1/8] leds: Add support for " Stefan Mavrodiev
2019-02-15 11:50   ` Stefan Mavrodiev
2019-02-15 11:50   ` Stefan Mavrodiev
2019-02-15 14:07   ` Stefan Mavrodiev
2019-02-15 14:07     ` Stefan Mavrodiev
2019-02-15 14:07     ` Stefan Mavrodiev
2019-02-15 15:57   ` Maxime Ripard
2019-02-15 15:57     ` Maxime Ripard
2019-02-15 15:57     ` Maxime Ripard
2019-02-15 18:32     ` Pavel Machek
2019-02-15 18:32       ` Pavel Machek
2019-02-15 18:32       ` Pavel Machek
2019-02-19  7:42       ` Stefan Mavrodiev
2019-02-19  7:42         ` Stefan Mavrodiev
2019-02-19  7:42         ` Stefan Mavrodiev
2019-02-15 11:50 ` [PATCH v2 2/8] mfd: axp20x: Add axp20x-led cell Stefan Mavrodiev
2019-02-15 11:50   ` Stefan Mavrodiev
2019-02-15 11:50   ` Stefan Mavrodiev
2019-03-25 10:01   ` Lee Jones
2019-03-25 10:01     ` Lee Jones
2019-03-25 10:01     ` Lee Jones
2019-02-15 11:50 ` [PATCH v2 3/8] dt-bindings: leds: Add binding for axp20x-led device driver Stefan Mavrodiev
2019-02-15 11:50   ` Stefan Mavrodiev
2019-02-15 11:50   ` Stefan Mavrodiev
2019-02-22 19:51   ` Rob Herring
2019-02-22 19:51     ` Rob Herring
2019-02-22 19:51     ` Rob Herring
2019-02-23 13:02     ` Jacek Anaszewski
2019-02-23 13:02       ` Jacek Anaszewski
2019-02-23 13:02       ` Jacek Anaszewski
2019-02-15 11:50 ` [PATCH v2 4/8] arm64: dts: allwinner: axp803: add charge led node Stefan Mavrodiev
2019-02-15 11:50   ` Stefan Mavrodiev
2019-02-15 11:50   ` Stefan Mavrodiev
2019-02-15 11:50 ` [PATCH v2 5/8] arm64: dts: allwinner: Enable AXP803 CHGLED for Olimex boards Stefan Mavrodiev
2019-02-15 11:50   ` Stefan Mavrodiev
2019-02-15 11:50   ` Stefan Mavrodiev
2019-02-15 18:49   ` Pavel Machek
2019-02-15 18:49     ` Pavel Machek
2019-02-15 18:49     ` Pavel Machek
2019-02-19  7:43     ` Stefan Mavrodiev
2019-02-19  7:43       ` Stefan Mavrodiev
2019-02-19  7:43       ` Stefan Mavrodiev
2019-02-15 11:50 ` [PATCH v2 6/8] arm: dts: axpxx: add charge led node Stefan Mavrodiev
2019-02-15 11:50   ` Stefan Mavrodiev
2019-02-15 11:50   ` Stefan Mavrodiev
2019-02-15 11:50 ` [PATCH v2 7/8] ARM: dts: sun7i: Enable AXP209 CHGLED for Olimex boards Stefan Mavrodiev
2019-02-15 11:50   ` Stefan Mavrodiev
2019-02-15 11:50   ` Stefan Mavrodiev
2019-02-15 11:50 ` [PATCH v2 8/8] ARM: dts: sun8i: a33: Enable AXP223 CHGLED for A33-OLinuXino Stefan Mavrodiev
2019-02-15 11:50   ` Stefan Mavrodiev
2019-02-15 11:50   ` Stefan Mavrodiev

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.