[PATCH 0/4] Add support for charger LED for AXP813 and TBS A711 Tablet

[PATCH 0/4] Add support for charger LED for AXP813 and TBS A711 Tablet

[Ondrej Jirman]

The tablet has a charger LED exposed on the top. This LED is controlled
by AXP813 PMIC. Add support for enabling the LED and using it either
for charging indication (handled by PMIC automatically) or for other uses
via user control.

Please take a look.

thank you and regards,
  Ondrej Jirman

Ondrej Jirman (4):
  dt-bindings: leds: Add a binding for AXP813 charger led
  leds: axp20x: Support charger LED on AXP20x like PMICs
  ARM: dts: axp813: Add charger LED
  ARM: dts: sun8i-a83t-tbs-a711: Enable charging LED

 .../devicetree/bindings/leds/leds-axp20x.yaml |  24 ++
 arch/arm/boot/dts/axp81x.dtsi                 |   5 +
 arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts     |   4 +
 drivers/leds/Kconfig                          |   7 +
 drivers/leds/Makefile                         |   1 +
 drivers/leds/leds-axp20x.c                    | 240 ++++++++++++++++++
 drivers/mfd/axp20x.c                          |   3 +
 7 files changed, 284 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-axp20x.yaml
 create mode 100644 drivers/leds/leds-axp20x.c

-- 
2.25.1

[PATCH 1/4] dt-bindings: leds: Add a binding for AXP813 charger led

[Ondrej Jirman]

The AXP813 PMIC can control one LED. Add binding to represent the LED.

Signed-off-by: Ondrej Jirman <megous@megous.com>
---
 .../devicetree/bindings/leds/leds-axp20x.yaml | 24 +++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-axp20x.yaml

diff --git a/Documentation/devicetree/bindings/leds/leds-axp20x.yaml b/Documentation/devicetree/bindings/leds/leds-axp20x.yaml
new file mode 100644
index 0000000000000..79282d55764bf
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-axp20x.yaml
@@ -0,0 +1,24 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/leds-axp20x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LED driver for AXP813 PMIC from X-Powers.
+
+maintainers:
+  - Ondrej Jirman <megous@megous.com>
+
+description: |
+  This module is part of the AXP20x MFD device. For more details
+  see Documentation/devicetree/bindings/mfd/axp20x.txt.
+
+  The LED controller is represented as a sub-node of the PMIC node on
+  the device tree.
+
+properties:
+  compatible:
+    const: x-powers,axp813-charger-led
+
+required:
+  - compatible
-- 
2.25.1

Re: [PATCH 1/4] dt-bindings: leds: Add a binding for AXP813 charger led

[Dan Murphy]

Ondrej

On 2/23/20 7:14 AM, Ondrej Jirman wrote:
> The AXP813 PMIC can control one LED. Add binding to represent the LED.
>
> Signed-off-by: Ondrej Jirman <megous@megous.com>
> ---
>   .../devicetree/bindings/leds/leds-axp20x.yaml | 24 +++++++++++++++++++
>   1 file changed, 24 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/leds/leds-axp20x.yaml
>
> diff --git a/Documentation/devicetree/bindings/leds/leds-axp20x.yaml b/Documentation/devicetree/bindings/leds/leds-axp20x.yaml
> new file mode 100644
> index 0000000000000..79282d55764bf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-axp20x.yaml
> @@ -0,0 +1,24 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/leds-axp20x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: LED driver for AXP813 PMIC from X-Powers.
> +
> +maintainers:
> +  - Ondrej Jirman <megous@megous.com>
> +
> +description: |
> +  This module is part of the AXP20x MFD device. For more details
> +  see Documentation/devicetree/bindings/mfd/axp20x.txt.
> +
> +  The LED controller is represented as a sub-node of the PMIC node on
> +  the device tree.
> +
> +properties:
> +  compatible:
> +    const: x-powers,axp813-charger-led
> +
> +required:
> +  - compatible

You need to add the color and function properties for proper LED device 
name presentation per the bindings/leds/common.yaml binding.

Dan

Re: [PATCH 1/4] dt-bindings: leds: Add a binding for AXP813 charger led

[Rob Herring]

On Sun, Feb 23, 2020 at 02:14:32PM +0100, Ondrej Jirman wrote:
> The AXP813 PMIC can control one LED. Add binding to represent the LED.
> 
> Signed-off-by: Ondrej Jirman <megous@megous.com>
> ---
>  .../devicetree/bindings/leds/leds-axp20x.yaml | 24 +++++++++++++++++++
>  1 file changed, 24 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-axp20x.yaml
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-axp20x.yaml b/Documentation/devicetree/bindings/leds/leds-axp20x.yaml
> new file mode 100644
> index 0000000000000..79282d55764bf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-axp20x.yaml
> @@ -0,0 +1,24 @@
> +# SPDX-License-Identifier: GPL-2.0

Dual license new bindings please:

(GPL-2.0-only OR BSD-2-Clause)

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/leds-axp20x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: LED driver for AXP813 PMIC from X-Powers.
> +
> +maintainers:
> +  - Ondrej Jirman <megous@megous.com>
> +
> +description: |
> +  This module is part of the AXP20x MFD device. For more details
> +  see Documentation/devicetree/bindings/mfd/axp20x.txt.

Really, we should convert this first as this should either just be 
referenced from the MFD schema or just directly put into it.

Rob

[PATCH 2/4] leds: axp20x: Support charger LED on AXP20x like PMICs

[Ondrej Jirman]

There is single LED that can be turned on and off by the user, or set to
be controlled by the charger in 2 different modes.

The driver initializes the LED to be controlled by the charger, but
allows to switch it to user control, and change the mode of charging
indication via a sysfs.

The driver was developed on AXP813, but should work on other PMICs like
that without changes.

Signed-off-by: Ondrej Jirman <megous@megous.com>
---
 drivers/leds/Kconfig       |   7 ++
 drivers/leds/Makefile      |   1 +
 drivers/leds/leds-axp20x.c | 240 +++++++++++++++++++++++++++++++++++++
 drivers/mfd/axp20x.c       |   3 +
 4 files changed, 251 insertions(+)
 create mode 100644 drivers/leds/leds-axp20x.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index d82f1dea37111..80a3f31f6f4c3 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -846,6 +846,13 @@ config LEDS_TPS6105X
 	  It is a single boost converter primarily for white LEDs and
 	  audio amplifiers.
 
+config LEDS_AXP20X
+	tristate "Charger LED support for AXP20X-like PMICs (AXP813, ...)"
+	depends on LEDS_CLASS && MFD_AXP20X
+	help
+	  This option enables support for on-chip LED driver on
+	  AXP20X-like PMICs.
+
 comment "LED Triggers"
 source "drivers/leds/trigger/Kconfig"
 
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index d7e1107753fb1..80ea1bc4744b0 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -86,6 +86,7 @@ obj-$(CONFIG_LEDS_TI_LMU_COMMON)	+= leds-ti-lmu-common.o
 obj-$(CONFIG_LEDS_LM3697)		+= leds-lm3697.o
 obj-$(CONFIG_LEDS_LM36274)		+= leds-lm36274.o
 obj-$(CONFIG_LEDS_TPS6105X)		+= leds-tps6105x.o
+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 0000000000000..e6c9853b84d52
--- /dev/null
+++ b/drivers/leds/leds-axp20x.c
@@ -0,0 +1,240 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * LED Driver for X-Powers AXP813 PMIC and similar.
+ *
+ * Copyright(c) 2020 Ondrej Jirman <megous@megous.com>
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/leds.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/mfd/axp20x.h>
+
+#define AXP20X_CHGLED_CTRL_MASK		BIT(3)
+#define AXP20X_CHGLED_CTRL_CHARGER	BIT(3)
+#define AXP20X_CHGLED_CTRL_USER		0
+
+#define AXP20X_CHRG_CTRL2_MODE		BIT(4)
+
+#define AXP20X_CHGLED_USER_STATE_MASK		GENMASK(5, 4)
+#define AXP20X_CHGLED_USER_STATE_OFF		(0 << 4)
+#define AXP20X_CHGLED_USER_STATE_BLINK_SLOW	(1 << 4)
+#define AXP20X_CHGLED_USER_STATE_BLINK_FAST	(2 << 4)
+#define AXP20X_CHGLED_USER_STATE_ON		(3 << 4)
+
+struct axp20x_led {
+	struct led_classdev cdev;
+	struct regmap *regmap;
+};
+
+static int axp20x_led_set(struct led_classdev *led_cdev,
+			   enum led_brightness value)
+{
+	struct axp20x_led *led =
+			container_of(led_cdev, struct axp20x_led, cdev);
+	unsigned int val;
+
+	val = value == LED_OFF ? AXP20X_CHGLED_USER_STATE_OFF :
+		AXP20X_CHGLED_USER_STATE_ON;
+
+	return regmap_update_bits(led->regmap, AXP20X_OFF_CTRL,
+				  AXP20X_CHGLED_USER_STATE_MASK, val);
+
+}
+
+static ssize_t charger_control_show(struct device *dev,
+				    struct device_attribute *attr, char *buf)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct axp20x_led *led =
+		container_of(led_cdev, struct axp20x_led, cdev);
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(led->regmap, AXP20X_OFF_CTRL, &val);
+	if (ret)
+		return ret;
+
+	return scnprintf(buf, PAGE_SIZE, "%u\n",
+			 val & AXP20X_CHGLED_CTRL_MASK ? 1 : 0);
+}
+
+static ssize_t charger_control_store(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t len)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct axp20x_led *led =
+			container_of(led_cdev, struct axp20x_led, cdev);
+	bool status;
+	int ret;
+
+	ret = kstrtobool(buf, &status);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(led->regmap, AXP20X_OFF_CTRL,
+				 AXP20X_CHGLED_CTRL_MASK,
+				 status ? AXP20X_CHGLED_CTRL_CHARGER :
+				 AXP20X_CHGLED_CTRL_USER);
+	if (ret)
+		return ret;
+
+	return len;
+}
+
+static ssize_t charger_mode_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct axp20x_led *led =
+		container_of(led_cdev, struct axp20x_led, cdev);
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(led->regmap, AXP20X_CHRG_CTRL2, &val);
+	if (ret)
+		return ret;
+
+	return scnprintf(buf, PAGE_SIZE, "%u\n",
+			 val & AXP20X_CHRG_CTRL2_MODE ? 1 : 0);
+}
+
+static ssize_t charger_mode_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t len)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct axp20x_led *led =
+		container_of(led_cdev, struct axp20x_led, cdev);
+	unsigned int mode;
+	int ret;
+
+	ret = kstrtouint(buf, 0, &mode);
+	if (ret)
+		return ret;
+
+	if (mode > 1)
+		return -ERANGE;
+
+	ret = regmap_update_bits(led->regmap, AXP20X_CHRG_CTRL2,
+				 AXP20X_CHRG_CTRL2_MODE,
+				 mode ? AXP20X_CHRG_CTRL2_MODE : 0);
+	if (ret)
+		return ret;
+
+	return len;
+}
+
+static DEVICE_ATTR_RW(charger_control);
+static DEVICE_ATTR_RW(charger_mode);
+
+static struct attribute *axp20x_led_attrs[] = {
+	&dev_attr_charger_control.attr,
+	&dev_attr_charger_mode.attr,
+	NULL,
+};
+
+ATTRIBUTE_GROUPS(axp20x_led);
+
+static int axp20x_led_probe(struct platform_device *pdev)
+{
+	struct axp20x_dev *axp20x;
+	struct axp20x_led *led;
+	int ret;
+
+	if (!of_device_is_available(pdev->dev.of_node))
+		return -ENODEV;
+
+	axp20x = dev_get_drvdata(pdev->dev.parent);
+	if (!axp20x)
+		return -EINVAL;
+
+	led = devm_kzalloc(&pdev->dev,
+			   sizeof(struct axp20x_led),
+			   GFP_KERNEL);
+	if (!led)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, led);
+
+	led->regmap = axp20x->regmap;
+
+	led->cdev.name = "axp20x-chgarger-led";
+	led->cdev.brightness_set_blocking = axp20x_led_set;
+	led->cdev.brightness = LED_OFF;
+	led->cdev.max_brightness = 1;
+	led->cdev.groups = axp20x_led_groups,
+
+	ret = devm_led_classdev_register(pdev->dev.parent, &led->cdev);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to register device %s\n",
+			led->cdev.name);
+		return ret;
+	}
+
+	ret = regmap_update_bits(led->regmap, AXP20X_OFF_CTRL,
+				 AXP20X_CHGLED_CTRL_MASK,
+				 AXP20X_CHGLED_CTRL_CHARGER);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to enable charger control\n");
+		return ret;
+	}
+
+	ret = axp20x_led_set(&led->cdev, led->cdev.brightness);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to init led brightness\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static void axp20x_led_shutdown(struct platform_device *pdev)
+{
+	struct axp20x_led *led = platform_get_drvdata(pdev);
+
+	/* On shutdown, we want to give LED control back to the PMIC,
+	 * so that it doesn't stay on, while the system is off.
+	 */
+
+	axp20x_led_set(&led->cdev, LED_OFF);
+
+	regmap_update_bits(led->regmap, AXP20X_OFF_CTRL,
+			   AXP20X_CHGLED_CTRL_MASK,
+			   AXP20X_CHGLED_CTRL_CHARGER);
+}
+
+static int axp20x_led_remove(struct platform_device *pdev)
+{
+	axp20x_led_shutdown(pdev);
+
+	return 0;
+}
+
+static const struct of_device_id axp20x_leds_of_match[] = {
+	{ .compatible = "x-powers,axp813-charger-led", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, axp20x_leds_of_match);
+
+static struct platform_driver axp20x_led_driver = {
+	.driver		= {
+		.name	= "leds-axp20x",
+		.of_match_table = axp20x_leds_of_match,
+	},
+	.probe		= axp20x_led_probe,
+	.remove		= axp20x_led_remove,
+	.shutdown	= axp20x_led_shutdown,
+};
+
+module_platform_driver(axp20x_led_driver);
+
+MODULE_AUTHOR("Ondrej Jirman <megous@megous.com>");
+MODULE_DESCRIPTION("LED driver for AXP813 PMIC");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:leds-axp20x");
diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index aa59496e43768..dcd68397b9d23 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -806,6 +806,9 @@ static const struct mfd_cell axp813_cells[] = {
 		.num_resources	= ARRAY_SIZE(axp803_usb_power_supply_resources),
 		.resources	= axp803_usb_power_supply_resources,
 		.of_compatible	= "x-powers,axp813-usb-power-supply",
+	}, {
+		.name		= "axp20x-charger-led",
+		.of_compatible	= "x-powers,axp813-charger-led",
 	},
 };
 
-- 
2.25.1

Re: [PATCH 2/4] leds: axp20x: Support charger LED on AXP20x like PMICs

[Dan Murphy]

Ondrej

On 2/23/20 7:14 AM, Ondrej Jirman wrote:
> There is single LED that can be turned on and off by the user, or set to
> be controlled by the charger in 2 different modes.
>
> The driver initializes the LED to be controlled by the charger, but
> allows to switch it to user control, and change the mode of charging
> indication via a sysfs.
>
> The driver was developed on AXP813, but should work on other PMICs like
> that without changes.
>
> Signed-off-by: Ondrej Jirman <megous@megous.com>
> ---
>   drivers/leds/Kconfig       |   7 ++
>   drivers/leds/Makefile      |   1 +
>   drivers/leds/leds-axp20x.c | 240 +++++++++++++++++++++++++++++++++++++
>   drivers/mfd/axp20x.c       |   3 +
>   4 files changed, 251 insertions(+)
>   create mode 100644 drivers/leds/leds-axp20x.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index d82f1dea37111..80a3f31f6f4c3 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -846,6 +846,13 @@ config LEDS_TPS6105X
>   	  It is a single boost converter primarily for white LEDs and
>   	  audio amplifiers.
>   
> +config LEDS_AXP20X
> +	tristate "Charger LED support for AXP20X-like PMICs (AXP813, ...)"
> +	depends on LEDS_CLASS && MFD_AXP20X
> +	help
> +	  This option enables support for on-chip LED driver on
> +	  AXP20X-like PMICs.
> +
>   comment "LED Triggers"
>   source "drivers/leds/trigger/Kconfig"
>   
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index d7e1107753fb1..80ea1bc4744b0 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -86,6 +86,7 @@ obj-$(CONFIG_LEDS_TI_LMU_COMMON)	+= leds-ti-lmu-common.o
>   obj-$(CONFIG_LEDS_LM3697)		+= leds-lm3697.o
>   obj-$(CONFIG_LEDS_LM36274)		+= leds-lm36274.o
>   obj-$(CONFIG_LEDS_TPS6105X)		+= leds-tps6105x.o
> +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 0000000000000..e6c9853b84d52
> --- /dev/null
> +++ b/drivers/leds/leds-axp20x.c
> @@ -0,0 +1,240 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * LED Driver for X-Powers AXP813 PMIC and similar.
> + *
> + * Copyright(c) 2020 Ondrej Jirman <megous@megous.com>
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/leds.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/mfd/axp20x.h>
> +
> +#define AXP20X_CHGLED_CTRL_MASK		BIT(3)
> +#define AXP20X_CHGLED_CTRL_CHARGER	BIT(3)
> +#define AXP20X_CHGLED_CTRL_USER		0
> +
> +#define AXP20X_CHRG_CTRL2_MODE		BIT(4)
> +
> +#define AXP20X_CHGLED_USER_STATE_MASK		GENMASK(5, 4)
> +#define AXP20X_CHGLED_USER_STATE_OFF		(0 << 4)
> +#define AXP20X_CHGLED_USER_STATE_BLINK_SLOW	(1 << 4)
> +#define AXP20X_CHGLED_USER_STATE_BLINK_FAST	(2 << 4)
> +#define AXP20X_CHGLED_USER_STATE_ON		(3 << 4)
> +
> +struct axp20x_led {
> +	struct led_classdev cdev;
> +	struct regmap *regmap;
> +};
> +
> +static int axp20x_led_set(struct led_classdev *led_cdev,
> +			   enum led_brightness value)
> +{
> +	struct axp20x_led *led =
> +			container_of(led_cdev, struct axp20x_led, cdev);
> +	unsigned int val;
> +
> +	val = value == LED_OFF ? AXP20X_CHGLED_USER_STATE_OFF :
> +		AXP20X_CHGLED_USER_STATE_ON;
> +
> +	return regmap_update_bits(led->regmap, AXP20X_OFF_CTRL,
> +				  AXP20X_CHGLED_USER_STATE_MASK, val);
> +
> +}
> +
> +static ssize_t charger_control_show(struct device *dev,
> +				    struct device_attribute *attr, char *buf)
> +{
> +	struct led_classdev *led_cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *led =
> +		container_of(led_cdev, struct axp20x_led, cdev);
> +	unsigned int val;
> +	int ret;
> +
> +	ret = regmap_read(led->regmap, AXP20X_OFF_CTRL, &val);
> +	if (ret)
> +		return ret;
> +
> +	return scnprintf(buf, PAGE_SIZE, "%u\n",
> +			 val & AXP20X_CHGLED_CTRL_MASK ? 1 : 0);
> +}
> +
> +static ssize_t charger_control_store(struct device *dev,
> +				     struct device_attribute *attr,
> +				     const char *buf, size_t len)
> +{
> +	struct led_classdev *led_cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *led =
> +			container_of(led_cdev, struct axp20x_led, cdev);
> +	bool status;
> +	int ret;
> +
> +	ret = kstrtobool(buf, &status);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_update_bits(led->regmap, AXP20X_OFF_CTRL,
> +				 AXP20X_CHGLED_CTRL_MASK,
> +				 status ? AXP20X_CHGLED_CTRL_CHARGER :
> +				 AXP20X_CHGLED_CTRL_USER);
> +	if (ret)
> +		return ret;
> +
> +	return len;
> +}
> +
> +static ssize_t charger_mode_show(struct device *dev,
> +				 struct device_attribute *attr, char *buf)
> +{
> +	struct led_classdev *led_cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *led =
> +		container_of(led_cdev, struct axp20x_led, cdev);
> +	unsigned int val;
> +	int ret;
> +
> +	ret = regmap_read(led->regmap, AXP20X_CHRG_CTRL2, &val);
> +	if (ret)
> +		return ret;
> +
> +	return scnprintf(buf, PAGE_SIZE, "%u\n",
> +			 val & AXP20X_CHRG_CTRL2_MODE ? 1 : 0);
> +}
> +
> +static ssize_t charger_mode_store(struct device *dev,
> +				  struct device_attribute *attr,
> +				  const char *buf, size_t len)
> +{
> +	struct led_classdev *led_cdev = dev_get_drvdata(dev);
> +	struct axp20x_led *led =
> +		container_of(led_cdev, struct axp20x_led, cdev);
> +	unsigned int mode;
> +	int ret;
> +
> +	ret = kstrtouint(buf, 0, &mode);
> +	if (ret)
> +		return ret;
> +
> +	if (mode > 1)
> +		return -ERANGE;
> +
> +	ret = regmap_update_bits(led->regmap, AXP20X_CHRG_CTRL2,
> +				 AXP20X_CHRG_CTRL2_MODE,
> +				 mode ? AXP20X_CHRG_CTRL2_MODE : 0);
> +	if (ret)
> +		return ret;
> +
> +	return len;
> +}
> +
> +static DEVICE_ATTR_RW(charger_control);
> +static DEVICE_ATTR_RW(charger_mode);
> +
> +static struct attribute *axp20x_led_attrs[] = {
> +	&dev_attr_charger_control.attr,
> +	&dev_attr_charger_mode.attr,
> +	NULL,
> +};
> +
> +ATTRIBUTE_GROUPS(axp20x_led);
> +
> +static int axp20x_led_probe(struct platform_device *pdev)
> +{
> +	struct axp20x_dev *axp20x;
> +	struct axp20x_led *led;
> +	int ret;
> +
> +	if (!of_device_is_available(pdev->dev.of_node))
> +		return -ENODEV;
> +
> +	axp20x = dev_get_drvdata(pdev->dev.parent);
> +	if (!axp20x)
> +		return -EINVAL;
> +
> +	led = devm_kzalloc(&pdev->dev,
> +			   sizeof(struct axp20x_led),
> +			   GFP_KERNEL);
> +	if (!led)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, led);
> +
> +	led->regmap = axp20x->regmap;
> +
> +	led->cdev.name = "axp20x-chgarger-led";


This does not follow the LED device naming convention please refer to 
the leds-class.rst document

https://elixir.bootlin.com/linux/latest/source/Documentation/leds/leds-class.rst

Dan

Re: [PATCH 2/4] leds: axp20x: Support charger LED on AXP20x like PMICs

[Pavel Machek]

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

Hi!

> There is single LED that can be turned on and off by the user, or set to
> be controlled by the charger in 2 different modes.
> 
> The driver initializes the LED to be controlled by the charger, but
> allows to switch it to user control, and change the mode of charging
> indication via a sysfs.

You'd need to document new "control" file.

But hold on, it is not an only driver with this "user-or-hardware"
feature, and we want to have a common solution. And that solution is
probably to have "hw-charger" trigger _just for this_ LED.

I can easily take a driver that always handles it as a "user" LED, if
you can get ACKs from device tree people on the documentation patches. 

> +	led->regmap = axp20x->regmap;
> +
> +	led->cdev.name = "axp20x-chgarger-led";

typo.

> +static void axp20x_led_shutdown(struct platform_device *pdev)
> +{
> +	struct axp20x_led *led = platform_get_drvdata(pdev);
> +
> +	/* On shutdown, we want to give LED control back to the PMIC,
> +	 * so that it doesn't stay on, while the system is off.
> +	 */

Comment style.

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

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

[PATCH 3/4] ARM: dts: axp813: Add charger LED

[Ondrej Jirman]

PMIC supports charging status indication via a LED. Add support
for it.

Signed-off-by: Ondrej Jirman <megous@megous.com>
---
 arch/arm/boot/dts/axp81x.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/axp81x.dtsi b/arch/arm/boot/dts/axp81x.dtsi
index 1dfeeceabf4c3..00b092f94433d 100644
--- a/arch/arm/boot/dts/axp81x.dtsi
+++ b/arch/arm/boot/dts/axp81x.dtsi
@@ -175,4 +175,9 @@ reg_drivevbus: drivevbus {
 	usb_power_supply: usb-power-supply {
 		compatible = "x-powers,axp813-usb-power-supply";
 	};
+
+	charger_led: charger-led {
+		compatible = "x-powers,axp813-charger-led";
+		status = "disabled";
+	};
 };
-- 
2.25.1

Re: [PATCH 3/4] ARM: dts: axp813: Add charger LED

[Dan Murphy]

Ondrej

On 2/23/20 7:14 AM, Ondrej Jirman wrote:
> PMIC supports charging status indication via a LED. Add support
> for it.
>
> Signed-off-by: Ondrej Jirman <megous@megous.com>
> ---
>   arch/arm/boot/dts/axp81x.dtsi | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm/boot/dts/axp81x.dtsi b/arch/arm/boot/dts/axp81x.dtsi
> index 1dfeeceabf4c3..00b092f94433d 100644
> --- a/arch/arm/boot/dts/axp81x.dtsi
> +++ b/arch/arm/boot/dts/axp81x.dtsi
> @@ -175,4 +175,9 @@ reg_drivevbus: drivevbus {
>   	usb_power_supply: usb-power-supply {
>   		compatible = "x-powers,axp813-usb-power-supply";
>   	};
> +
> +	charger_led: charger-led {
> +		compatible = "x-powers,axp813-charger-led";
> +		status = "disabled";

As I commented before you need to add function and color to this node 
and read it from the driver.  Or you can add them to the over ride for 
the specific use case.

Another question is is this LED only used for charging or can it be 
multi purpose?

Dan


> +	};
>   };

[PATCH 4/4] ARM: dts: sun8i-a83t-tbs-a711: Enable charging LED

[Ondrej Jirman]

The tablet has a LED connected to the PMIC. The LED is visible in the
top right corner of the tablet. Enable it.

Signed-off-by: Ondrej Jirman <megous@megous.com>
---
 arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts b/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts
index 2fd31a0a0b344..40911b5d3f323 100644
--- a/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts
+++ b/arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts
@@ -300,6 +300,10 @@ &battery_power_supply {
 	status = "okay";
 };
 
+&charger_led {
+	status = "okay";
+};
+
 &reg_aldo1 {
 	regulator-min-microvolt = <1800000>;
 	regulator-max-microvolt = <1800000>;
-- 
2.25.1

Re: [PATCH 0/4] Add support for charger LED for AXP813 and TBS A711 Tablet

[Ondřej Jirman]

On Sun, Feb 23, 2020 at 02:14:31PM +0100, megous hlavni wrote:
> The tablet has a charger LED exposed on the top. This LED is controlled
> by AXP813 PMIC. Add support for enabling the LED and using it either
> for charging indication (handled by PMIC automatically) or for other uses
> via user control.

Dang, I just noticed someone sent a similar driver recently, although I had this
one prepared for quite some time (since 2017) in my tree. I guess I should have
sent it earlier.

Please ignore.

regards,
	o.


> Please take a look.
> 
> thank you and regards,
>   Ondrej Jirman
> 
> Ondrej Jirman (4):
>   dt-bindings: leds: Add a binding for AXP813 charger led
>   leds: axp20x: Support charger LED on AXP20x like PMICs
>   ARM: dts: axp813: Add charger LED
>   ARM: dts: sun8i-a83t-tbs-a711: Enable charging LED
> 
>  .../devicetree/bindings/leds/leds-axp20x.yaml |  24 ++
>  arch/arm/boot/dts/axp81x.dtsi                 |   5 +
>  arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts     |   4 +
>  drivers/leds/Kconfig                          |   7 +
>  drivers/leds/Makefile                         |   1 +
>  drivers/leds/leds-axp20x.c                    | 240 ++++++++++++++++++
>  drivers/mfd/axp20x.c                          |   3 +
>  7 files changed, 284 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-axp20x.yaml
>  create mode 100644 drivers/leds/leds-axp20x.c
> 
> -- 
> 2.25.1
> 

Re: [PATCH 0/4] Add support for charger LED for AXP813 and TBS A711 Tablet

[Ondřej Jirman]

On Sun, Feb 23, 2020 at 02:27:30PM +0100, megous hlavni wrote:
> On Sun, Feb 23, 2020 at 02:14:31PM +0100, megous hlavni wrote:
> > The tablet has a charger LED exposed on the top. This LED is controlled
> > by AXP813 PMIC. Add support for enabling the LED and using it either
> > for charging indication (handled by PMIC automatically) or for other uses
> > via user control.
> 
> Dang, I just noticed someone sent a similar driver recently, although I had this
> one prepared for quite some time (since 2017) in my tree. I guess I should have
> sent it earlier.
> 
> Please ignore.

Though the meaning of "recently" is a bit relative. The other work was sent in
a year ago. Here's a reference:

  https://lore.kernel.org/patchwork/cover/1042764/

> regards,
> 	o.
> 
> 
> > Please take a look.
> > 
> > thank you and regards,
> >   Ondrej Jirman
> > 
> > Ondrej Jirman (4):
> >   dt-bindings: leds: Add a binding for AXP813 charger led
> >   leds: axp20x: Support charger LED on AXP20x like PMICs
> >   ARM: dts: axp813: Add charger LED
> >   ARM: dts: sun8i-a83t-tbs-a711: Enable charging LED
> > 
> >  .../devicetree/bindings/leds/leds-axp20x.yaml |  24 ++
> >  arch/arm/boot/dts/axp81x.dtsi                 |   5 +
> >  arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts     |   4 +
> >  drivers/leds/Kconfig                          |   7 +
> >  drivers/leds/Makefile                         |   1 +
> >  drivers/leds/leds-axp20x.c                    | 240 ++++++++++++++++++
> >  drivers/mfd/axp20x.c                          |   3 +
> >  7 files changed, 284 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/leds/leds-axp20x.yaml
> >  create mode 100644 drivers/leds/leds-axp20x.c
> > 
> > -- 
> > 2.25.1
> > 

Re: [linux-sunxi] Re: [PATCH 0/4] Add support for charger LED for AXP813 and TBS A711 Tablet

[Stefan Mavrodiev]

On 2/23/20 3:35 PM, Ondřej Jirman wrote:
> On Sun, Feb 23, 2020 at 02:27:30PM +0100, megous hlavni wrote:
>> On Sun, Feb 23, 2020 at 02:14:31PM +0100, megous hlavni wrote:
>>> The tablet has a charger LED exposed on the top. This LED is controlled
>>> by AXP813 PMIC. Add support for enabling the LED and using it either
>>> for charging indication (handled by PMIC automatically) or for other uses
>>> via user control.
>> Dang, I just noticed someone sent a similar driver recently, although I had this
>> one prepared for quite some time (since 2017) in my tree. I guess I should have
>> sent it earlier.
>>
>> Please ignore.
> Though the meaning of "recently" is a bit relative. The other work was sent in
> a year ago. Here's a reference:
>
>    https://lore.kernel.org/patchwork/cover/1042764/

Hi,

I'm the author of the 'other' work. I don't know the full story here, 
but I don't
mind someone else submitting this patch as his.

When I submitted the last patch, there was the proposal to use the 
ledtrig-pattern instead
of sysfs entries. Also AXP209 has inverted CTRL bit.

Please read the the 'other' discussion.

Best regards,
Stefan Mavrodiev

>
>> regards,
>> 	o.
>>
>>
>>> Please take a look.
>>>
>>> thank you and regards,
>>>    Ondrej Jirman
>>>
>>> Ondrej Jirman (4):
>>>    dt-bindings: leds: Add a binding for AXP813 charger led
>>>    leds: axp20x: Support charger LED on AXP20x like PMICs
>>>    ARM: dts: axp813: Add charger LED
>>>    ARM: dts: sun8i-a83t-tbs-a711: Enable charging LED
>>>
>>>   .../devicetree/bindings/leds/leds-axp20x.yaml |  24 ++
>>>   arch/arm/boot/dts/axp81x.dtsi                 |   5 +
>>>   arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts     |   4 +
>>>   drivers/leds/Kconfig                          |   7 +
>>>   drivers/leds/Makefile                         |   1 +
>>>   drivers/leds/leds-axp20x.c                    | 240 ++++++++++++++++++
>>>   drivers/mfd/axp20x.c                          |   3 +
>>>   7 files changed, 284 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/leds/leds-axp20x.yaml
>>>   create mode 100644 drivers/leds/leds-axp20x.c
>>>
>>> -- 
>>> 2.25.1
>>>

Re: [linux-sunxi] Re: [PATCH 0/4] Add support for charger LED for AXP813 and TBS A711 Tablet

[Ondřej Jirman]

On Mon, Feb 24, 2020 at 08:31:06AM +0200, Stefan Mavrodiev wrote:
> 
> On 2/23/20 3:35 PM, Ondřej Jirman wrote:
> > On Sun, Feb 23, 2020 at 02:27:30PM +0100, megous hlavni wrote:
> > > On Sun, Feb 23, 2020 at 02:14:31PM +0100, megous hlavni wrote:
> > > > The tablet has a charger LED exposed on the top. This LED is controlled
> > > > by AXP813 PMIC. Add support for enabling the LED and using it either
> > > > for charging indication (handled by PMIC automatically) or for other uses
> > > > via user control.
> > > Dang, I just noticed someone sent a similar driver recently, although I had this
> > > one prepared for quite some time (since 2017) in my tree. I guess I should have
> > > sent it earlier.
> > > 
> > > Please ignore.
> > Though the meaning of "recently" is a bit relative. The other work was sent in
> > a year ago. Here's a reference:
> > 
> >    https://lore.kernel.org/patchwork/cover/1042764/
> 
> Hi,
> 
> I'm the author of the 'other' work. I don't know the full story here, but I
> don't
> mind someone else submitting this patch as his.

Hello Stefan,

There's really no story. Just me being a bit anoyed at myself, for not checking
the mailing lists prior to spending some time cleaning up and extending some old
patches to upstream them, and wasting quite a bit of time in the process.

https://megous.com/git/linux/commit/?h=linux-tbs&id=737eec64565d328cab98b75879e3f9eb1cf2f609

> When I submitted the last patch, there was the proposal to use the
> ledtrig-pattern instead
> of sysfs entries. Also AXP209 has inverted CTRL bit.
> 
> Please read the the 'other' discussion.

Thanks, I'll check it out.

regards,
	o.

> Best regards,
> Stefan Mavrodiev
> 
> > 
> > > regards,
> > > 	o.
> > > 
> > > 
> > > > Please take a look.
> > > > 
> > > > thank you and regards,
> > > >    Ondrej Jirman
> > > > 
> > > > Ondrej Jirman (4):
> > > >    dt-bindings: leds: Add a binding for AXP813 charger led
> > > >    leds: axp20x: Support charger LED on AXP20x like PMICs
> > > >    ARM: dts: axp813: Add charger LED
> > > >    ARM: dts: sun8i-a83t-tbs-a711: Enable charging LED
> > > > 
> > > >   .../devicetree/bindings/leds/leds-axp20x.yaml |  24 ++
> > > >   arch/arm/boot/dts/axp81x.dtsi                 |   5 +
> > > >   arch/arm/boot/dts/sun8i-a83t-tbs-a711.dts     |   4 +
> > > >   drivers/leds/Kconfig                          |   7 +
> > > >   drivers/leds/Makefile                         |   1 +
> > > >   drivers/leds/leds-axp20x.c                    | 240 ++++++++++++++++++
> > > >   drivers/mfd/axp20x.c                          |   3 +
> > > >   7 files changed, 284 insertions(+)
> > > >   create mode 100644 Documentation/devicetree/bindings/leds/leds-axp20x.yaml
> > > >   create mode 100644 drivers/leds/leds-axp20x.c
> > > > 
> > > > -- 
> > > > 2.25.1
> > > > 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel