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
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 | 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 <[email protected]>
+
+#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 <[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..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
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 <[email protected]>
---
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>;
+};
+
®_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>;
+};
+
®_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
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
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 <[email protected]>
---
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>;
+};
+
®_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>;
+};
+
®_aldo1 {
regulator-always-on;
regulator-min-microvolt = <2800000>;
--
2.17.1
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 <[email protected]>
---
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
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
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 <[email protected]>
> ---
> 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 <[email protected]>
> +
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 <[email protected]");
> +MODULE_DESCRIPTION("X-Powers PMIC CHGLED driver");
> +MODULE_LICENSE("GPL");
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
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
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 <[email protected]>
> ---
> 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
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
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 <[email protected]>
>> ---
>> 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
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 <[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..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
>
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 <[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..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
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 <[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" },
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 <[email protected]>
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog