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 <[email protected]>
---
drivers/leds/Kconfig | 10 ++
drivers/leds/Makefile | 1 +
drivers/leds/leds-axp20x.c | 283 +++++++++++++++++++++++++++++++++++++
3 files changed, 294 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..9c03410833a3
--- /dev/null
+++ b/drivers/leds/leds-axp20x.c
@@ -0,0 +1,283 @@
+// SPDX-License-Identifier: GPL-2.0+
+//
+// Copyright 2019 Stefan Mavrodiev <[email protected]>
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/leds.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#include <linux/mfd/axp20x.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 {
+ struct led_classdev cdev;
+ enum led_brightness current_brightness;
+ 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;
+
+ ret = regmap_update_bits(priv->axp20x->regmap, AXP20X_CHGLED_CTRL_REG,
+ AXP20X_CHGLED_CTRL_MASK,
+ AXP20X_CHGLED_CTRL(val));
+ if (ret < 0)
+ return ret;
+
+ return regmap_update_bits(priv->axp20x->regmap, AXP20X_CHGLED_MODE_REG,
+ AXP20X_CHGLED_MODE_MASK,
+ AXP20X_CHGLED_MODE(priv->mode));
+}
+
+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, "%d\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, "%d\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;
+
+ ret = regmap_read(priv->axp20x->regmap, AXP20X_CHGLED_CTRL_REG, &val);
+ 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;
+
+ if (!priv->current_brightness && brightness)
+ ret = regmap_update_bits(priv->axp20x->regmap,
+ AXP20X_CHGLED_CTRL_REG,
+ AXP20X_CHGLED_FUNC_MASK,
+ AXP20X_CHGLED_FUNC_FULL);
+ else if (priv->current_brightness && !brightness)
+ ret = regmap_update_bits(priv->axp20x->regmap,
+ AXP20X_CHGLED_CTRL_REG,
+ AXP20X_CHGLED_FUNC_MASK,
+ AXP20X_CHGLED_FUNC_OFF);
+
+ if (ret < 0)
+ return ret;
+
+ priv->current_brightness = brightness;
+ return 0;
+}
+
+static int axp20x_led_parse_dt(struct axp20x_led *priv, struct device_node *np)
+{
+ const char *state;
+ int ret = 0;
+ u8 value;
+
+ priv->cdev.name = of_get_property(np, "label", NULL) ? : np->name;
+
+ 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;
+ }
+
+ priv->cdev.default_trigger = of_get_property(np,
+ "linux,default-trigger",
+ NULL);
+
+ state = of_get_property(np, "default-state", NULL);
+ if (state) {
+ if (!strcmp(state, "keep")) {
+ ret = axp20x_led_brightness_get(&priv->cdev);
+ if (ret < 0)
+ return ret;
+ priv->current_brightness = ret;
+ } else if (!strcmp(state, "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;
+ }
+
+ priv->cdev.max_brightness = LED_FULL;
+ 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 <[email protected]");
+MODULE_DESCRIPTION("X-Powers PMIC CHGLED driver");
+MODULE_LICENSE("GPL");
--
2.17.1
Add axp20x-led cell for AXP20x, AXP221, AXP223, AXP228, AXP803, AXP809
and AXP813.
Signed-off-by: Stefan Mavrodiev <[email protected]>
---
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
This adds the devicetree bindings for charge led indicator found
on most of X-Powers AXP20X PMICs.
Signed-off-by: Stefan Mavrodiev <[email protected]>
---
.../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..73fae22ffe29
--- /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 subnode of AXP20X MFD driver, since it uses shared bus with all
+other cells.
+The LED can be controlled either manually or automatic. 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 leds properties, see ./common.txt for more information:
+- label : sets LED label. If omitted, dt device node is used
+- linux,default-trigger : See
+- default-state:
+
+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";
+
+ ...
+
+ axp20x-led {
+ 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";
+
+ ...
+
+ axp20x-led {
+ compatible = "x-powers,axp20x-led";
+ status = "okay";
+
+ label = "axp20x:yellow:chgled";
+ x-powers,charger-mode = "mode-b";
+ };
+ };
--
2.17.1
Add dt node for axp20x-led driver controlling CHGLED.
Default status is disabled, since it may be not used.
Signed-off-by: Stefan Mavrodiev <[email protected]>
---
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
Add dt node for axp20x-led driver controlling CHGLED.
Default status is disabled, since it may be not used.
Signed-off-by: Stefan Mavrodiev <[email protected]>
---
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
Hi Stefan,
Thank you for the patch.
I have few nits below.
On 1/31/19 9:23 AM, 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 <[email protected]>
> ---
> .../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..73fae22ffe29
> --- /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 subnode of AXP20X MFD driver, since it uses shared bus with all
s/is subnode/is a subnode/
> +other cells.
> +The LED can be controlled either manually or automatic. Then in automatic
s/or automatic/or automatically/
> +(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 leds properties, see ./common.txt for more information:
s/leds/LED/
Please remove ", see ./common.txt for more information" in favor
of references indicated below.
> +- label : sets LED label. If omitted, dt device node is used
Please change it to:
- label : See Documentation/devicetree/bindings/leds/common.txt
> +- linux,default-trigger : See
> +- default-state:
- linux,default-trigger : 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";
> +
> + ...
> +
> + axp20x-led {
s/axp20x-led/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";
> +
> + ...
> +
> + axp20x-led {
s/axp20x-led/led@0/
> + compatible = "x-powers,axp20x-led";
> + status = "okay";
> +
> + label = "axp20x:yellow:chgled";
> + x-powers,charger-mode = "mode-b";
> + };
> + };
>
--
Best regards,
Jacek Anaszewski
Hi Stefan,
Thank you for the patch.
On 1/31/19 9:23 AM, 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 <[email protected]>
> ---
> drivers/leds/Kconfig | 10 ++
> drivers/leds/Makefile | 1 +
> drivers/leds/leds-axp20x.c | 283 +++++++++++++++++++++++++++++++++++++
> 3 files changed, 294 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..9c03410833a3
> --- /dev/null
> +++ b/drivers/leds/leds-axp20x.c
> @@ -0,0 +1,283 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +//
> +// Copyright 2019 Stefan Mavrodiev <[email protected]>
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/leds.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +#include <linux/mfd/axp20x.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 {
> + struct led_classdev cdev;
> + enum led_brightness current_brightness;
You don't need this. Please use the one from struct led_classdev.
> + 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;
You need mutex protection in all places where the hardware is accessed.
It is possible that brightness_set_blocking() will be called from
trigger e.g. after first regmap_update_bits_below().
> +
> + ret = regmap_update_bits(priv->axp20x->regmap, AXP20X_CHGLED_CTRL_REG,
> + AXP20X_CHGLED_CTRL_MASK,
> + AXP20X_CHGLED_CTRL(val));
> + if (ret < 0)
> + return ret;
> +
> + return regmap_update_bits(priv->axp20x->regmap, AXP20X_CHGLED_MODE_REG,
> + AXP20X_CHGLED_MODE_MASK,
> + AXP20X_CHGLED_MODE(priv->mode));
> +}
> +
> +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, "%d\n", priv->ctrl);
s/%d/%u/
> +}
> +
> +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, "%d\n", priv->mode);
Ditto.
> +}
> +
> +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;
> +
> + ret = regmap_read(priv->axp20x->regmap, AXP20X_CHGLED_CTRL_REG, &val);
> + 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;
> +
> + if (!priv->current_brightness && brightness)
> + ret = regmap_update_bits(priv->axp20x->regmap,
> + AXP20X_CHGLED_CTRL_REG,
> + AXP20X_CHGLED_FUNC_MASK,
> + AXP20X_CHGLED_FUNC_FULL);
> + else if (priv->current_brightness && !brightness)
> + ret = regmap_update_bits(priv->axp20x->regmap,
> + AXP20X_CHGLED_CTRL_REG,
> + AXP20X_CHGLED_FUNC_MASK,
> + AXP20X_CHGLED_FUNC_OFF);
> +
> + if (ret < 0)
> + return ret;
> +
> + priv->current_brightness = brightness;
> + return 0;
> +}
> +
> +static int axp20x_led_parse_dt(struct axp20x_led *priv, struct device_node *np)
> +{
> + const char *state;
> + int ret = 0;
> + u8 value;
> +
> + priv->cdev.name = of_get_property(np, "label", NULL) ? : np->name;
We now compose LED names differently. Please refer e.g. to:
drivers/leds/leds-cr0014114.c.
> +
> + 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;
> + }
> +
> + priv->cdev.default_trigger = of_get_property(np,
> + "linux,default-trigger",
> + NULL);
> +
> + state = of_get_property(np, "default-state", NULL);
> + if (state) {
> + if (!strcmp(state, "keep")) {
> + ret = axp20x_led_brightness_get(&priv->cdev);
> + if (ret < 0)
> + return ret;
> + priv->current_brightness = ret;
> + } else if (!strcmp(state, "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;
> + }
> +
> + priv->cdev.max_brightness = LED_FULL;
LED core will initialize it to LED_FULL when set to 0 by kzalloc().
> + 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 <[email protected]");
> +MODULE_DESCRIPTION("X-Powers PMIC CHGLED driver");
> +MODULE_LICENSE("GPL");
>
--
Best regards,
Jacek Anaszewski
On Thu, Jan 31, 2019 at 4:25 PM Stefan Mavrodiev <[email protected]> wrote:
>
> Add dt node for axp20x-led driver controlling CHGLED.
> Default status is disabled, since it may be not used.
>
> Signed-off-by: Stefan Mavrodiev <[email protected]>
Please include a cover letter for such a patch series.
Also, do any boards actually use this? I know the Pine64 does, but the
LED is left to the user to populate. I would really like to have an in
kernel user of this function, so that we can verify it.
ChenYu
> ---
> 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
>
On 2/5/19 6:16 PM, Chen-Yu Tsai wrote:
> On Thu, Jan 31, 2019 at 4:25 PM Stefan Mavrodiev <[email protected]> wrote:
>> Add dt node for axp20x-led driver controlling CHGLED.
>> Default status is disabled, since it may be not used.
>>
>> Signed-off-by: Stefan Mavrodiev <[email protected]>
> Please include a cover letter for such a patch series.
Sorry, I will.
>
> Also, do any boards actually use this? I know the Pine64 does, but the
> LED is left to the user to populate. I would really like to have an in
> kernel user of this function, so that we can verify it.
All boards made by Olimex has this led populated. The development and the
testing of the driver was done on A64-OLinuXino board.
Should I make another patch, enabling the LED for the mention board?
Best regards,
Stefan Mavrodiev
>
> ChenYu
>
>> ---
>> 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
>>
On Wed, Feb 6, 2019 at 2:06 PM Stefan Mavrodiev <[email protected]> wrote:
>
>
> On 2/5/19 6:16 PM, Chen-Yu Tsai wrote:
> > On Thu, Jan 31, 2019 at 4:25 PM Stefan Mavrodiev <[email protected]> wrote:
> >> Add dt node for axp20x-led driver controlling CHGLED.
> >> Default status is disabled, since it may be not used.
> >>
> >> Signed-off-by: Stefan Mavrodiev <[email protected]>
> > Please include a cover letter for such a patch series.
> Sorry, I will.
> >
> > Also, do any boards actually use this? I know the Pine64 does, but the
> > LED is left to the user to populate. I would really like to have an in
> > kernel user of this function, so that we can verify it.
>
> All boards made by Olimex has this led populated. The development and the
> testing of the driver was done on A64-OLinuXino board.
>
> Should I make another patch, enabling the LED for the mention board?
Yes please. I'll dig out my A33-OLinuXino next week to try it out.
ChenYu
> >
> > ChenYu
> >
> >> ---
> >> 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
> >>