* [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
@ 2016-01-08 2:13 ` Wenyou Yang
0 siblings, 0 replies; 29+ messages in thread
From: Wenyou Yang @ 2016-01-08 2:13 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds new driver for Active-semi ACT8945A ActivePath
charger (part of ACT8945A MFD driver) providing power supply class
information to userspace.
The driver is configured through DTS (battery and system related
settings) and sysfs entries (timers and input over-voltage threshold).
Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---
drivers/power/Kconfig | 7 +
drivers/power/Makefile | 1 +
drivers/power/act8945a_charger.c | 397 ++++++++++++++++++++++++++++++++++++++
3 files changed, 405 insertions(+)
create mode 100644 drivers/power/act8945a_charger.c
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 1ddd13c..ae75211 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -75,6 +75,13 @@ config BATTERY_88PM860X
help
Say Y here to enable battery monitor for Marvell 88PM860x chip.
+config BATTERY_ACT8945A
+ tristate "Active-semi ACT8945A charger driver"
+ depends on MFD_ACT8945A
+ help
+ Say Y here to enable support for power supply provided by
+ Active-semi ActivePath ACT8945A charger.
+
config BATTERY_DS2760
tristate "DS2760 battery driver (HP iPAQ & others)"
depends on W1 && W1_SLAVE_DS2760
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index 0e4eab5..e46b75d 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_WM8350_POWER) += wm8350_power.o
obj-$(CONFIG_TEST_POWER) += test_power.o
obj-$(CONFIG_BATTERY_88PM860X) += 88pm860x_battery.o
+obj-$(CONFIG_BATTERY_ACT8945A) += act8945a_charger.o
obj-$(CONFIG_BATTERY_DS2760) += ds2760_battery.o
obj-$(CONFIG_BATTERY_DS2780) += ds2780_battery.o
obj-$(CONFIG_BATTERY_DS2781) += ds2781_battery.o
diff --git a/drivers/power/act8945a_charger.c b/drivers/power/act8945a_charger.c
new file mode 100644
index 0000000..5b55cd7
--- /dev/null
+++ b/drivers/power/act8945a_charger.c
@@ -0,0 +1,397 @@
+/*
+ * Power supply driver for the Active-semi ACT8945A PMIC
+ *
+ * Copyright (C) 2015 Atmel Corporation
+ * Wenyou Yang <wenyou.yang@atmel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/mfd/act8945a.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/regmap.h>
+
+static const char *act8945a_charger_model = "ACT8945A";
+static const char *act8945a_charger_manufacturer = "Active-semi";
+
+/**
+ * ACT8945A Charger Register Map
+ */
+
+/* 0x70: Reserved */
+#define ACT8945A_APCH_CFG 0x71
+#define ACT8945A_APCH_STATUS 0x78
+#define ACT8945A_APCH_CTRL 0x79
+#define ACT8945A_APCH_STATE 0x7A
+
+/* ACT8945A_APCH_CFG */
+#define APCH_CFG_OVPSET (0x03 << 0)
+#define APCH_CFG_OVPSET_6V6 (0x0 << 0)
+#define APCH_CFG_OVPSET_7V (0x1 << 0)
+#define APCH_CFG_OVPSET_7V5 (0x2 << 0)
+#define APCH_CFG_OVPSET_8V (0x3 << 0)
+#define APCH_CFG_PRETIMO (0x03 << 2)
+#define APCH_CFG_PRETIMO_40_MIN (0x0 << 2)
+#define APCH_CFG_PRETIMO_60_MIN (0x1 << 2)
+#define APCH_CFG_PRETIMO_80_MIN (0x2 << 2)
+#define APCH_CFG_PRETIMO_DISABLED (0x3 << 2)
+#define APCH_CFG_TOTTIMO (0x03 << 4)
+#define APCH_CFG_TOTTIMO_3_HOUR (0x0 << 4)
+#define APCH_CFG_TOTTIMO_4_HOUR (0x1 << 4)
+#define APCH_CFG_TOTTIMO_5_HOUR (0x2 << 4)
+#define APCH_CFG_TOTTIMO_DISABLED (0x3 << 4)
+#define APCH_CFG_SUSCHG (0x01 << 7)
+
+#define APCH_STATUS_CHGDAT (0x01 << 0)
+#define APCH_STATUS_INDAT (0x01 << 1)
+#define APCH_STATUS_TEMPDAT (0x01 << 2)
+#define APCH_STATUS_TIMRDAT (0x01 << 3)
+#define APCH_STATUS_CHGSTAT (0x01 << 4)
+#define APCH_STATUS_INSTAT (0x01 << 5)
+#define APCH_STATUS_TEMPSTAT (0x01 << 6)
+#define APCH_STATUS_TIMRSTAT (0x01 << 7)
+
+#define APCH_CTRL_CHGEOCOUT (0x01 << 0)
+#define APCH_CTRL_INDIS (0x01 << 1)
+#define APCH_CTRL_TEMPOUT (0x01 << 2)
+#define APCH_CTRL_TIMRPRE (0x01 << 3)
+#define APCH_CTRL_CHGEOCIN (0x01 << 4)
+#define APCH_CTRL_INCON (0x01 << 5)
+#define APCH_CTRL_TEMPIN (0x01 << 6)
+#define APCH_CTRL_TIMRTOT (0x01 << 7)
+
+#define APCH_STATE_ACINSTAT (0x01 << 1)
+#define APCH_STATE_CSTATE (0x03 << 4)
+#define APCH_STATE_CSTATE_SHIFT 4
+#define APCH_STATE_CSTATE_DISABLED 0x00
+#define APCH_STATE_CSTATE_EOC 0x01
+#define APCH_STATE_CSTATE_FAST 0x02
+#define APCH_STATE_CSTATE_PRE 0x03
+
+struct act8945a_charger {
+ struct device *dev;
+ struct act8945a_dev *act8945a_dev;
+ struct power_supply *psy;
+
+ u32 tolal_time_out;
+ u32 pre_time_out;
+ u32 input_voltage_threshold;
+ bool battery_temperature;
+ int chglev_pin;
+ int chglev_value;
+};
+
+static int act8945a_get_charger_state(struct regmap *regmap, int *val)
+{
+ int ret;
+ unsigned int status, state;
+
+ ret = regmap_read(regmap, ACT8945A_APCH_STATUS, &status);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_read(regmap, ACT8945A_APCH_STATE, &state);
+ if (ret < 0)
+ return ret;
+
+ state &= APCH_STATE_CSTATE;
+ state >>= APCH_STATE_CSTATE_SHIFT;
+
+ if (state == APCH_STATE_CSTATE_EOC) {
+ if (status & APCH_STATUS_CHGDAT)
+ *val = POWER_SUPPLY_STATUS_FULL;
+ else
+ *val = POWER_SUPPLY_STATUS_NOT_CHARGING;
+ } else if ((state == APCH_STATE_CSTATE_FAST) ||
+ (state == APCH_STATE_CSTATE_PRE)) {
+ *val = POWER_SUPPLY_STATUS_CHARGING;
+ } else {
+ *val = POWER_SUPPLY_STATUS_NOT_CHARGING;
+ }
+
+ return 0;
+}
+
+static int act8945a_get_charge_type(struct regmap *regmap, int *val)
+{
+ int ret;
+ unsigned int state;
+
+ ret = regmap_read(regmap, ACT8945A_APCH_STATE, &state);
+ if (ret < 0)
+ return ret;
+
+ state &= APCH_STATE_CSTATE;
+ state >>= APCH_STATE_CSTATE_SHIFT;
+
+ switch (state) {
+ case APCH_STATE_CSTATE_PRE:
+ *val = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
+ break;
+ case APCH_STATE_CSTATE_FAST:
+ *val = POWER_SUPPLY_CHARGE_TYPE_FAST;
+ break;
+ case APCH_STATE_CSTATE_EOC:
+ case APCH_STATE_CSTATE_DISABLED:
+ default:
+ *val = POWER_SUPPLY_CHARGE_TYPE_NONE;
+ }
+
+ return 0;
+}
+
+static int act8945a_get_battery_health(struct act8945a_charger *charger,
+ struct regmap *regmap, int *val)
+{
+ int ret;
+ unsigned int status;
+
+ ret = regmap_read(regmap, ACT8945A_APCH_STATUS, &status);
+ if (ret < 0)
+ return ret;
+
+ if (charger->battery_temperature && !(status & APCH_STATUS_TEMPDAT))
+ *val = POWER_SUPPLY_HEALTH_OVERHEAT;
+ else if (!(status & APCH_STATUS_INDAT))
+ *val = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+ else if (status & APCH_STATUS_TIMRDAT)
+ *val = POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE;
+ else
+ *val = POWER_SUPPLY_HEALTH_GOOD;
+
+ return 0;
+}
+
+static enum power_supply_property act8945a_charger_props[] = {
+ POWER_SUPPLY_PROP_STATUS,
+ POWER_SUPPLY_PROP_CHARGE_TYPE,
+ POWER_SUPPLY_PROP_TECHNOLOGY,
+ POWER_SUPPLY_PROP_HEALTH,
+ POWER_SUPPLY_PROP_MODEL_NAME,
+ POWER_SUPPLY_PROP_MANUFACTURER
+};
+
+static int act8945a_charger_get_property(struct power_supply *psy,
+ enum power_supply_property prop,
+ union power_supply_propval *val)
+{
+ struct act8945a_charger *charger = power_supply_get_drvdata(psy);
+ struct regmap *regmap = charger->act8945a_dev->regmap;
+ int ret = 0;
+
+ switch (prop) {
+ case POWER_SUPPLY_PROP_STATUS:
+ ret = act8945a_get_charger_state(regmap, &val->intval);
+ break;
+ case POWER_SUPPLY_PROP_CHARGE_TYPE:
+ ret = act8945a_get_charge_type(regmap, &val->intval);
+ break;
+ case POWER_SUPPLY_PROP_TECHNOLOGY:
+ val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
+ break;
+ case POWER_SUPPLY_PROP_HEALTH:
+ ret = act8945a_get_battery_health(charger,
+ regmap, &val->intval);
+ break;
+ case POWER_SUPPLY_PROP_MODEL_NAME:
+ val->strval = act8945a_charger_model;
+ break;
+ case POWER_SUPPLY_PROP_MANUFACTURER:
+ val->strval = act8945a_charger_manufacturer;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return ret;
+}
+
+static const struct power_supply_desc act8945a_charger_desc = {
+ .name = "act8945a-charger",
+ .type = POWER_SUPPLY_TYPE_BATTERY,
+ .get_property = act8945a_charger_get_property,
+ .properties = act8945a_charger_props,
+ .num_properties = ARRAY_SIZE(act8945a_charger_props),
+};
+
+#define DEFAULT_TOTAL_TIME_OUT 3
+#define DEFAULT_PRE_TIME_OUT 40
+#define DEFAULT_INPUT_OVP_THRESHOLD 6600
+
+static int act8945a_charger_parse_dt(struct device *dev,
+ struct act8945a_charger *charger)
+{
+ struct device_node *np = dev->of_node;
+ enum of_gpio_flags flags;
+
+ if (!np) {
+ dev_err(dev, "no charger of node\n");
+ return -EINVAL;
+ }
+
+ charger->chglev_pin = of_get_named_gpio_flags(np,
+ "active-semi,chglev-gpio", 0, &flags);
+
+ charger->chglev_value = (flags == OF_GPIO_ACTIVE_LOW) ? 0 : 1;
+
+ charger->battery_temperature = !!of_get_property(np,
+ "active-semi,battery_temperature", NULL);
+
+ if (of_property_read_u32(np, "active-semi,input_voltage_threshold",
+ &charger->input_voltage_threshold))
+ charger->input_voltage_threshold = DEFAULT_PRE_TIME_OUT;
+
+ if (of_property_read_u32(np, "active-semi,precondition_timeout",
+ &charger->pre_time_out))
+ charger->pre_time_out = DEFAULT_PRE_TIME_OUT;
+
+ if (of_property_read_u32(np, "active-semi,total_timeout",
+ &charger->tolal_time_out))
+ charger->tolal_time_out = DEFAULT_TOTAL_TIME_OUT;
+
+ return 0;
+}
+
+static int act8945a_charger_config(struct act8945a_charger *charger)
+{
+ struct regmap *regmap = charger->act8945a_dev->regmap;
+ u8 value = 0;
+
+ if (gpio_is_valid(charger->chglev_pin))
+ gpio_set_value(charger->chglev_pin, charger->chglev_value);
+
+ switch (charger->input_voltage_threshold) {
+ case 8000:
+ value |= APCH_CFG_OVPSET_8V;
+ break;
+ case 7500:
+ value |= APCH_CFG_OVPSET_7V5;
+ break;
+ case 7000:
+ value |= APCH_CFG_OVPSET_7V;
+ break;
+ case 6600:
+ default:
+ value |= APCH_CFG_OVPSET_6V6;
+ break;
+ }
+
+ switch (charger->pre_time_out) {
+ case 60:
+ value |= APCH_CFG_PRETIMO_60_MIN;
+ break;
+ case 80:
+ value |= APCH_CFG_PRETIMO_80_MIN;
+ break;
+ case 0:
+ value |= APCH_CFG_PRETIMO_DISABLED;
+ break;
+ case 40:
+ default:
+ value |= APCH_CFG_PRETIMO_40_MIN;
+ break;
+ }
+
+ switch (charger->tolal_time_out) {
+ case 4:
+ value |= APCH_CFG_TOTTIMO_4_HOUR;
+ break;
+ case 5:
+ value |= APCH_CFG_TOTTIMO_5_HOUR;
+ break;
+ case 0:
+ value |= APCH_CFG_TOTTIMO_DISABLED;
+ break;
+ case 3:
+ default:
+ value |= APCH_CFG_TOTTIMO_3_HOUR;
+ break;
+ }
+
+ return regmap_write(regmap, ACT8945A_APCH_CFG, value);
+}
+
+static int act8945a_charger_probe(struct platform_device *pdev)
+{
+ struct act8945a_dev *act8945a_dev = dev_get_drvdata(pdev->dev.parent);
+ struct act8945a_charger *charger;
+ struct power_supply_config psy_cfg = {};
+ int ret;
+
+ charger = devm_kzalloc(&pdev->dev, sizeof(*charger), GFP_KERNEL);
+ if (!charger)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, charger);
+
+ charger->dev = &pdev->dev;
+ charger->act8945a_dev = act8945a_dev;
+
+ ret = act8945a_charger_parse_dt(&pdev->dev, charger);
+ if (ret)
+ return ret;
+
+ ret = act8945a_charger_config(charger);
+ if (ret)
+ return ret;
+
+ psy_cfg.of_node = pdev->dev.of_node;
+ psy_cfg.drv_data = charger;
+
+ charger->psy = power_supply_register(&pdev->dev,
+ &act8945a_charger_desc,
+ &psy_cfg);
+ if (IS_ERR(charger->psy)) {
+ dev_err(act8945a_dev->dev, "failed to register power supply\n");
+ return PTR_ERR(charger->psy);
+ }
+
+ dev_info(act8945a_dev->dev, "charger driver registered\n");
+
+ return 0;
+}
+
+static int act8945a_charger_remove(struct platform_device *pdev)
+{
+ struct act8945a_charger *charger = platform_get_drvdata(pdev);
+
+ power_supply_unregister(charger->psy);
+
+ return 0;
+}
+
+static struct platform_driver act8945a_charger_driver = {
+ .driver = {
+ .name = "act8945a-charger",
+ },
+ .probe = act8945a_charger_probe,
+ .remove = act8945a_charger_remove,
+};
+
+static int __init act8945a_charger_init(void)
+{
+ return platform_driver_register(&act8945a_charger_driver);
+}
+subsys_initcall(act8945a_charger_init);
+
+static void __exit act8945a_charger_cleanup(void)
+{
+ platform_driver_unregister(&act8945a_charger_driver);
+}
+module_exit(act8945a_charger_cleanup);
+
+MODULE_DESCRIPTION("Active-semi ACT8945A ActivePath charger driver");
+MODULE_AUTHOR("Wenyou Yang <wenyou.yang@atmel.com>");
+MODULE_LICENSE("GPL v2");
--
1.7.9.5
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
@ 2016-01-08 2:13 ` Wenyou Yang
0 siblings, 0 replies; 29+ messages in thread
From: Wenyou Yang @ 2016-01-08 2:13 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Nicolas Ferre, linux-arm-kernel, linux-kernel, linux-pm
Cc: Wenyou Yang
This patch adds new driver for Active-semi ACT8945A ActivePath
charger (part of ACT8945A MFD driver) providing power supply class
information to userspace.
The driver is configured through DTS (battery and system related
settings) and sysfs entries (timers and input over-voltage threshold).
Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---
drivers/power/Kconfig | 7 +
drivers/power/Makefile | 1 +
drivers/power/act8945a_charger.c | 397 ++++++++++++++++++++++++++++++++++++++
3 files changed, 405 insertions(+)
create mode 100644 drivers/power/act8945a_charger.c
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 1ddd13c..ae75211 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -75,6 +75,13 @@ config BATTERY_88PM860X
help
Say Y here to enable battery monitor for Marvell 88PM860x chip.
+config BATTERY_ACT8945A
+ tristate "Active-semi ACT8945A charger driver"
+ depends on MFD_ACT8945A
+ help
+ Say Y here to enable support for power supply provided by
+ Active-semi ActivePath ACT8945A charger.
+
config BATTERY_DS2760
tristate "DS2760 battery driver (HP iPAQ & others)"
depends on W1 && W1_SLAVE_DS2760
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index 0e4eab5..e46b75d 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_WM8350_POWER) += wm8350_power.o
obj-$(CONFIG_TEST_POWER) += test_power.o
obj-$(CONFIG_BATTERY_88PM860X) += 88pm860x_battery.o
+obj-$(CONFIG_BATTERY_ACT8945A) += act8945a_charger.o
obj-$(CONFIG_BATTERY_DS2760) += ds2760_battery.o
obj-$(CONFIG_BATTERY_DS2780) += ds2780_battery.o
obj-$(CONFIG_BATTERY_DS2781) += ds2781_battery.o
diff --git a/drivers/power/act8945a_charger.c b/drivers/power/act8945a_charger.c
new file mode 100644
index 0000000..5b55cd7
--- /dev/null
+++ b/drivers/power/act8945a_charger.c
@@ -0,0 +1,397 @@
+/*
+ * Power supply driver for the Active-semi ACT8945A PMIC
+ *
+ * Copyright (C) 2015 Atmel Corporation
+ * Wenyou Yang <wenyou.yang@atmel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/mfd/act8945a.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/regmap.h>
+
+static const char *act8945a_charger_model = "ACT8945A";
+static const char *act8945a_charger_manufacturer = "Active-semi";
+
+/**
+ * ACT8945A Charger Register Map
+ */
+
+/* 0x70: Reserved */
+#define ACT8945A_APCH_CFG 0x71
+#define ACT8945A_APCH_STATUS 0x78
+#define ACT8945A_APCH_CTRL 0x79
+#define ACT8945A_APCH_STATE 0x7A
+
+/* ACT8945A_APCH_CFG */
+#define APCH_CFG_OVPSET (0x03 << 0)
+#define APCH_CFG_OVPSET_6V6 (0x0 << 0)
+#define APCH_CFG_OVPSET_7V (0x1 << 0)
+#define APCH_CFG_OVPSET_7V5 (0x2 << 0)
+#define APCH_CFG_OVPSET_8V (0x3 << 0)
+#define APCH_CFG_PRETIMO (0x03 << 2)
+#define APCH_CFG_PRETIMO_40_MIN (0x0 << 2)
+#define APCH_CFG_PRETIMO_60_MIN (0x1 << 2)
+#define APCH_CFG_PRETIMO_80_MIN (0x2 << 2)
+#define APCH_CFG_PRETIMO_DISABLED (0x3 << 2)
+#define APCH_CFG_TOTTIMO (0x03 << 4)
+#define APCH_CFG_TOTTIMO_3_HOUR (0x0 << 4)
+#define APCH_CFG_TOTTIMO_4_HOUR (0x1 << 4)
+#define APCH_CFG_TOTTIMO_5_HOUR (0x2 << 4)
+#define APCH_CFG_TOTTIMO_DISABLED (0x3 << 4)
+#define APCH_CFG_SUSCHG (0x01 << 7)
+
+#define APCH_STATUS_CHGDAT (0x01 << 0)
+#define APCH_STATUS_INDAT (0x01 << 1)
+#define APCH_STATUS_TEMPDAT (0x01 << 2)
+#define APCH_STATUS_TIMRDAT (0x01 << 3)
+#define APCH_STATUS_CHGSTAT (0x01 << 4)
+#define APCH_STATUS_INSTAT (0x01 << 5)
+#define APCH_STATUS_TEMPSTAT (0x01 << 6)
+#define APCH_STATUS_TIMRSTAT (0x01 << 7)
+
+#define APCH_CTRL_CHGEOCOUT (0x01 << 0)
+#define APCH_CTRL_INDIS (0x01 << 1)
+#define APCH_CTRL_TEMPOUT (0x01 << 2)
+#define APCH_CTRL_TIMRPRE (0x01 << 3)
+#define APCH_CTRL_CHGEOCIN (0x01 << 4)
+#define APCH_CTRL_INCON (0x01 << 5)
+#define APCH_CTRL_TEMPIN (0x01 << 6)
+#define APCH_CTRL_TIMRTOT (0x01 << 7)
+
+#define APCH_STATE_ACINSTAT (0x01 << 1)
+#define APCH_STATE_CSTATE (0x03 << 4)
+#define APCH_STATE_CSTATE_SHIFT 4
+#define APCH_STATE_CSTATE_DISABLED 0x00
+#define APCH_STATE_CSTATE_EOC 0x01
+#define APCH_STATE_CSTATE_FAST 0x02
+#define APCH_STATE_CSTATE_PRE 0x03
+
+struct act8945a_charger {
+ struct device *dev;
+ struct act8945a_dev *act8945a_dev;
+ struct power_supply *psy;
+
+ u32 tolal_time_out;
+ u32 pre_time_out;
+ u32 input_voltage_threshold;
+ bool battery_temperature;
+ int chglev_pin;
+ int chglev_value;
+};
+
+static int act8945a_get_charger_state(struct regmap *regmap, int *val)
+{
+ int ret;
+ unsigned int status, state;
+
+ ret = regmap_read(regmap, ACT8945A_APCH_STATUS, &status);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_read(regmap, ACT8945A_APCH_STATE, &state);
+ if (ret < 0)
+ return ret;
+
+ state &= APCH_STATE_CSTATE;
+ state >>= APCH_STATE_CSTATE_SHIFT;
+
+ if (state == APCH_STATE_CSTATE_EOC) {
+ if (status & APCH_STATUS_CHGDAT)
+ *val = POWER_SUPPLY_STATUS_FULL;
+ else
+ *val = POWER_SUPPLY_STATUS_NOT_CHARGING;
+ } else if ((state == APCH_STATE_CSTATE_FAST) ||
+ (state == APCH_STATE_CSTATE_PRE)) {
+ *val = POWER_SUPPLY_STATUS_CHARGING;
+ } else {
+ *val = POWER_SUPPLY_STATUS_NOT_CHARGING;
+ }
+
+ return 0;
+}
+
+static int act8945a_get_charge_type(struct regmap *regmap, int *val)
+{
+ int ret;
+ unsigned int state;
+
+ ret = regmap_read(regmap, ACT8945A_APCH_STATE, &state);
+ if (ret < 0)
+ return ret;
+
+ state &= APCH_STATE_CSTATE;
+ state >>= APCH_STATE_CSTATE_SHIFT;
+
+ switch (state) {
+ case APCH_STATE_CSTATE_PRE:
+ *val = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
+ break;
+ case APCH_STATE_CSTATE_FAST:
+ *val = POWER_SUPPLY_CHARGE_TYPE_FAST;
+ break;
+ case APCH_STATE_CSTATE_EOC:
+ case APCH_STATE_CSTATE_DISABLED:
+ default:
+ *val = POWER_SUPPLY_CHARGE_TYPE_NONE;
+ }
+
+ return 0;
+}
+
+static int act8945a_get_battery_health(struct act8945a_charger *charger,
+ struct regmap *regmap, int *val)
+{
+ int ret;
+ unsigned int status;
+
+ ret = regmap_read(regmap, ACT8945A_APCH_STATUS, &status);
+ if (ret < 0)
+ return ret;
+
+ if (charger->battery_temperature && !(status & APCH_STATUS_TEMPDAT))
+ *val = POWER_SUPPLY_HEALTH_OVERHEAT;
+ else if (!(status & APCH_STATUS_INDAT))
+ *val = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+ else if (status & APCH_STATUS_TIMRDAT)
+ *val = POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE;
+ else
+ *val = POWER_SUPPLY_HEALTH_GOOD;
+
+ return 0;
+}
+
+static enum power_supply_property act8945a_charger_props[] = {
+ POWER_SUPPLY_PROP_STATUS,
+ POWER_SUPPLY_PROP_CHARGE_TYPE,
+ POWER_SUPPLY_PROP_TECHNOLOGY,
+ POWER_SUPPLY_PROP_HEALTH,
+ POWER_SUPPLY_PROP_MODEL_NAME,
+ POWER_SUPPLY_PROP_MANUFACTURER
+};
+
+static int act8945a_charger_get_property(struct power_supply *psy,
+ enum power_supply_property prop,
+ union power_supply_propval *val)
+{
+ struct act8945a_charger *charger = power_supply_get_drvdata(psy);
+ struct regmap *regmap = charger->act8945a_dev->regmap;
+ int ret = 0;
+
+ switch (prop) {
+ case POWER_SUPPLY_PROP_STATUS:
+ ret = act8945a_get_charger_state(regmap, &val->intval);
+ break;
+ case POWER_SUPPLY_PROP_CHARGE_TYPE:
+ ret = act8945a_get_charge_type(regmap, &val->intval);
+ break;
+ case POWER_SUPPLY_PROP_TECHNOLOGY:
+ val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
+ break;
+ case POWER_SUPPLY_PROP_HEALTH:
+ ret = act8945a_get_battery_health(charger,
+ regmap, &val->intval);
+ break;
+ case POWER_SUPPLY_PROP_MODEL_NAME:
+ val->strval = act8945a_charger_model;
+ break;
+ case POWER_SUPPLY_PROP_MANUFACTURER:
+ val->strval = act8945a_charger_manufacturer;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return ret;
+}
+
+static const struct power_supply_desc act8945a_charger_desc = {
+ .name = "act8945a-charger",
+ .type = POWER_SUPPLY_TYPE_BATTERY,
+ .get_property = act8945a_charger_get_property,
+ .properties = act8945a_charger_props,
+ .num_properties = ARRAY_SIZE(act8945a_charger_props),
+};
+
+#define DEFAULT_TOTAL_TIME_OUT 3
+#define DEFAULT_PRE_TIME_OUT 40
+#define DEFAULT_INPUT_OVP_THRESHOLD 6600
+
+static int act8945a_charger_parse_dt(struct device *dev,
+ struct act8945a_charger *charger)
+{
+ struct device_node *np = dev->of_node;
+ enum of_gpio_flags flags;
+
+ if (!np) {
+ dev_err(dev, "no charger of node\n");
+ return -EINVAL;
+ }
+
+ charger->chglev_pin = of_get_named_gpio_flags(np,
+ "active-semi,chglev-gpio", 0, &flags);
+
+ charger->chglev_value = (flags == OF_GPIO_ACTIVE_LOW) ? 0 : 1;
+
+ charger->battery_temperature = !!of_get_property(np,
+ "active-semi,battery_temperature", NULL);
+
+ if (of_property_read_u32(np, "active-semi,input_voltage_threshold",
+ &charger->input_voltage_threshold))
+ charger->input_voltage_threshold = DEFAULT_PRE_TIME_OUT;
+
+ if (of_property_read_u32(np, "active-semi,precondition_timeout",
+ &charger->pre_time_out))
+ charger->pre_time_out = DEFAULT_PRE_TIME_OUT;
+
+ if (of_property_read_u32(np, "active-semi,total_timeout",
+ &charger->tolal_time_out))
+ charger->tolal_time_out = DEFAULT_TOTAL_TIME_OUT;
+
+ return 0;
+}
+
+static int act8945a_charger_config(struct act8945a_charger *charger)
+{
+ struct regmap *regmap = charger->act8945a_dev->regmap;
+ u8 value = 0;
+
+ if (gpio_is_valid(charger->chglev_pin))
+ gpio_set_value(charger->chglev_pin, charger->chglev_value);
+
+ switch (charger->input_voltage_threshold) {
+ case 8000:
+ value |= APCH_CFG_OVPSET_8V;
+ break;
+ case 7500:
+ value |= APCH_CFG_OVPSET_7V5;
+ break;
+ case 7000:
+ value |= APCH_CFG_OVPSET_7V;
+ break;
+ case 6600:
+ default:
+ value |= APCH_CFG_OVPSET_6V6;
+ break;
+ }
+
+ switch (charger->pre_time_out) {
+ case 60:
+ value |= APCH_CFG_PRETIMO_60_MIN;
+ break;
+ case 80:
+ value |= APCH_CFG_PRETIMO_80_MIN;
+ break;
+ case 0:
+ value |= APCH_CFG_PRETIMO_DISABLED;
+ break;
+ case 40:
+ default:
+ value |= APCH_CFG_PRETIMO_40_MIN;
+ break;
+ }
+
+ switch (charger->tolal_time_out) {
+ case 4:
+ value |= APCH_CFG_TOTTIMO_4_HOUR;
+ break;
+ case 5:
+ value |= APCH_CFG_TOTTIMO_5_HOUR;
+ break;
+ case 0:
+ value |= APCH_CFG_TOTTIMO_DISABLED;
+ break;
+ case 3:
+ default:
+ value |= APCH_CFG_TOTTIMO_3_HOUR;
+ break;
+ }
+
+ return regmap_write(regmap, ACT8945A_APCH_CFG, value);
+}
+
+static int act8945a_charger_probe(struct platform_device *pdev)
+{
+ struct act8945a_dev *act8945a_dev = dev_get_drvdata(pdev->dev.parent);
+ struct act8945a_charger *charger;
+ struct power_supply_config psy_cfg = {};
+ int ret;
+
+ charger = devm_kzalloc(&pdev->dev, sizeof(*charger), GFP_KERNEL);
+ if (!charger)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, charger);
+
+ charger->dev = &pdev->dev;
+ charger->act8945a_dev = act8945a_dev;
+
+ ret = act8945a_charger_parse_dt(&pdev->dev, charger);
+ if (ret)
+ return ret;
+
+ ret = act8945a_charger_config(charger);
+ if (ret)
+ return ret;
+
+ psy_cfg.of_node = pdev->dev.of_node;
+ psy_cfg.drv_data = charger;
+
+ charger->psy = power_supply_register(&pdev->dev,
+ &act8945a_charger_desc,
+ &psy_cfg);
+ if (IS_ERR(charger->psy)) {
+ dev_err(act8945a_dev->dev, "failed to register power supply\n");
+ return PTR_ERR(charger->psy);
+ }
+
+ dev_info(act8945a_dev->dev, "charger driver registered\n");
+
+ return 0;
+}
+
+static int act8945a_charger_remove(struct platform_device *pdev)
+{
+ struct act8945a_charger *charger = platform_get_drvdata(pdev);
+
+ power_supply_unregister(charger->psy);
+
+ return 0;
+}
+
+static struct platform_driver act8945a_charger_driver = {
+ .driver = {
+ .name = "act8945a-charger",
+ },
+ .probe = act8945a_charger_probe,
+ .remove = act8945a_charger_remove,
+};
+
+static int __init act8945a_charger_init(void)
+{
+ return platform_driver_register(&act8945a_charger_driver);
+}
+subsys_initcall(act8945a_charger_init);
+
+static void __exit act8945a_charger_cleanup(void)
+{
+ platform_driver_unregister(&act8945a_charger_driver);
+}
+module_exit(act8945a_charger_cleanup);
+
+MODULE_DESCRIPTION("Active-semi ACT8945A ActivePath charger driver");
+MODULE_AUTHOR("Wenyou Yang <wenyou.yang@atmel.com>");
+MODULE_LICENSE("GPL v2");
--
1.7.9.5
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
2016-01-08 2:13 ` Wenyou Yang
@ 2016-01-08 3:55 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 29+ messages in thread
From: Krzysztof Kozlowski @ 2016-01-08 3:55 UTC (permalink / raw)
To: Wenyou Yang
Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Nicolas Ferre, linux-arm-kernel, linux-kernel, linux-pm
2016-01-08 11:13 GMT+09:00 Wenyou Yang <wenyou.yang@atmel.com>:
> This patch adds new driver for Active-semi ACT8945A ActivePath
> charger (part of ACT8945A MFD driver) providing power supply class
> information to userspace.
>
> The driver is configured through DTS (battery and system related
> settings) and sysfs entries (timers and input over-voltage threshold).
>
> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> ---
>
> drivers/power/Kconfig | 7 +
> drivers/power/Makefile | 1 +
> drivers/power/act8945a_charger.c | 397 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 405 insertions(+)
> create mode 100644 drivers/power/act8945a_charger.c
>
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 1ddd13c..ae75211 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -75,6 +75,13 @@ config BATTERY_88PM860X
> help
> Say Y here to enable battery monitor for Marvell 88PM860x chip.
>
> +config BATTERY_ACT8945A
> + tristate "Active-semi ACT8945A charger driver"
> + depends on MFD_ACT8945A
> + help
> + Say Y here to enable support for power supply provided by
> + Active-semi ActivePath ACT8945A charger.
> +
> config BATTERY_DS2760
> tristate "DS2760 battery driver (HP iPAQ & others)"
> depends on W1 && W1_SLAVE_DS2760
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index 0e4eab5..e46b75d 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -17,6 +17,7 @@ obj-$(CONFIG_WM8350_POWER) += wm8350_power.o
> obj-$(CONFIG_TEST_POWER) += test_power.o
>
> obj-$(CONFIG_BATTERY_88PM860X) += 88pm860x_battery.o
> +obj-$(CONFIG_BATTERY_ACT8945A) += act8945a_charger.o
> obj-$(CONFIG_BATTERY_DS2760) += ds2760_battery.o
> obj-$(CONFIG_BATTERY_DS2780) += ds2780_battery.o
> obj-$(CONFIG_BATTERY_DS2781) += ds2781_battery.o
> diff --git a/drivers/power/act8945a_charger.c b/drivers/power/act8945a_charger.c
> new file mode 100644
> index 0000000..5b55cd7
> --- /dev/null
> +++ b/drivers/power/act8945a_charger.c
> @@ -0,0 +1,397 @@
> +/*
> + * Power supply driver for the Active-semi ACT8945A PMIC
> + *
> + * Copyright (C) 2015 Atmel Corporation
> + * Wenyou Yang <wenyou.yang@atmel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/mfd/act8945a.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +#include <linux/regmap.h>
> +
> +static const char *act8945a_charger_model = "ACT8945A";
> +static const char *act8945a_charger_manufacturer = "Active-semi";
> +
> +/**
> + * ACT8945A Charger Register Map
> + */
> +
> +/* 0x70: Reserved */
> +#define ACT8945A_APCH_CFG 0x71
> +#define ACT8945A_APCH_STATUS 0x78
> +#define ACT8945A_APCH_CTRL 0x79
> +#define ACT8945A_APCH_STATE 0x7A
> +
> +/* ACT8945A_APCH_CFG */
> +#define APCH_CFG_OVPSET (0x03 << 0)
> +#define APCH_CFG_OVPSET_6V6 (0x0 << 0)
> +#define APCH_CFG_OVPSET_7V (0x1 << 0)
> +#define APCH_CFG_OVPSET_7V5 (0x2 << 0)
> +#define APCH_CFG_OVPSET_8V (0x3 << 0)
> +#define APCH_CFG_PRETIMO (0x03 << 2)
> +#define APCH_CFG_PRETIMO_40_MIN (0x0 << 2)
> +#define APCH_CFG_PRETIMO_60_MIN (0x1 << 2)
> +#define APCH_CFG_PRETIMO_80_MIN (0x2 << 2)
> +#define APCH_CFG_PRETIMO_DISABLED (0x3 << 2)
> +#define APCH_CFG_TOTTIMO (0x03 << 4)
> +#define APCH_CFG_TOTTIMO_3_HOUR (0x0 << 4)
> +#define APCH_CFG_TOTTIMO_4_HOUR (0x1 << 4)
> +#define APCH_CFG_TOTTIMO_5_HOUR (0x2 << 4)
> +#define APCH_CFG_TOTTIMO_DISABLED (0x3 << 4)
> +#define APCH_CFG_SUSCHG (0x01 << 7)
> +
> +#define APCH_STATUS_CHGDAT (0x01 << 0)
> +#define APCH_STATUS_INDAT (0x01 << 1)
> +#define APCH_STATUS_TEMPDAT (0x01 << 2)
> +#define APCH_STATUS_TIMRDAT (0x01 << 3)
> +#define APCH_STATUS_CHGSTAT (0x01 << 4)
> +#define APCH_STATUS_INSTAT (0x01 << 5)
> +#define APCH_STATUS_TEMPSTAT (0x01 << 6)
> +#define APCH_STATUS_TIMRSTAT (0x01 << 7)
> +
> +#define APCH_CTRL_CHGEOCOUT (0x01 << 0)
> +#define APCH_CTRL_INDIS (0x01 << 1)
> +#define APCH_CTRL_TEMPOUT (0x01 << 2)
> +#define APCH_CTRL_TIMRPRE (0x01 << 3)
> +#define APCH_CTRL_CHGEOCIN (0x01 << 4)
> +#define APCH_CTRL_INCON (0x01 << 5)
> +#define APCH_CTRL_TEMPIN (0x01 << 6)
> +#define APCH_CTRL_TIMRTOT (0x01 << 7)
> +
> +#define APCH_STATE_ACINSTAT (0x01 << 1)
> +#define APCH_STATE_CSTATE (0x03 << 4)
> +#define APCH_STATE_CSTATE_SHIFT 4
> +#define APCH_STATE_CSTATE_DISABLED 0x00
> +#define APCH_STATE_CSTATE_EOC 0x01
> +#define APCH_STATE_CSTATE_FAST 0x02
> +#define APCH_STATE_CSTATE_PRE 0x03
> +
> +struct act8945a_charger {
> + struct device *dev;
> + struct act8945a_dev *act8945a_dev;
> + struct power_supply *psy;
> +
> + u32 tolal_time_out;
> + u32 pre_time_out;
> + u32 input_voltage_threshold;
> + bool battery_temperature;
> + int chglev_pin;
> + int chglev_value;
> +};
> +
> +static int act8945a_get_charger_state(struct regmap *regmap, int *val)
> +{
> + int ret;
> + unsigned int status, state;
> +
> + ret = regmap_read(regmap, ACT8945A_APCH_STATUS, &status);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_read(regmap, ACT8945A_APCH_STATE, &state);
> + if (ret < 0)
> + return ret;
> +
> + state &= APCH_STATE_CSTATE;
> + state >>= APCH_STATE_CSTATE_SHIFT;
> +
> + if (state == APCH_STATE_CSTATE_EOC) {
> + if (status & APCH_STATUS_CHGDAT)
> + *val = POWER_SUPPLY_STATUS_FULL;
> + else
> + *val = POWER_SUPPLY_STATUS_NOT_CHARGING;
> + } else if ((state == APCH_STATE_CSTATE_FAST) ||
> + (state == APCH_STATE_CSTATE_PRE)) {
> + *val = POWER_SUPPLY_STATUS_CHARGING;
> + } else {
> + *val = POWER_SUPPLY_STATUS_NOT_CHARGING;
> + }
> +
> + return 0;
> +}
> +
> +static int act8945a_get_charge_type(struct regmap *regmap, int *val)
> +{
> + int ret;
> + unsigned int state;
> +
> + ret = regmap_read(regmap, ACT8945A_APCH_STATE, &state);
> + if (ret < 0)
> + return ret;
> +
> + state &= APCH_STATE_CSTATE;
> + state >>= APCH_STATE_CSTATE_SHIFT;
> +
> + switch (state) {
> + case APCH_STATE_CSTATE_PRE:
> + *val = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
> + break;
> + case APCH_STATE_CSTATE_FAST:
> + *val = POWER_SUPPLY_CHARGE_TYPE_FAST;
> + break;
> + case APCH_STATE_CSTATE_EOC:
> + case APCH_STATE_CSTATE_DISABLED:
> + default:
> + *val = POWER_SUPPLY_CHARGE_TYPE_NONE;
> + }
> +
> + return 0;
> +}
> +
> +static int act8945a_get_battery_health(struct act8945a_charger *charger,
> + struct regmap *regmap, int *val)
> +{
> + int ret;
> + unsigned int status;
> +
> + ret = regmap_read(regmap, ACT8945A_APCH_STATUS, &status);
> + if (ret < 0)
> + return ret;
> +
> + if (charger->battery_temperature && !(status & APCH_STATUS_TEMPDAT))
> + *val = POWER_SUPPLY_HEALTH_OVERHEAT;
> + else if (!(status & APCH_STATUS_INDAT))
> + *val = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
> + else if (status & APCH_STATUS_TIMRDAT)
> + *val = POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE;
> + else
> + *val = POWER_SUPPLY_HEALTH_GOOD;
> +
> + return 0;
> +}
> +
> +static enum power_supply_property act8945a_charger_props[] = {
> + POWER_SUPPLY_PROP_STATUS,
> + POWER_SUPPLY_PROP_CHARGE_TYPE,
> + POWER_SUPPLY_PROP_TECHNOLOGY,
> + POWER_SUPPLY_PROP_HEALTH,
> + POWER_SUPPLY_PROP_MODEL_NAME,
> + POWER_SUPPLY_PROP_MANUFACTURER
> +};
> +
> +static int act8945a_charger_get_property(struct power_supply *psy,
> + enum power_supply_property prop,
> + union power_supply_propval *val)
> +{
> + struct act8945a_charger *charger = power_supply_get_drvdata(psy);
> + struct regmap *regmap = charger->act8945a_dev->regmap;
> + int ret = 0;
> +
> + switch (prop) {
> + case POWER_SUPPLY_PROP_STATUS:
> + ret = act8945a_get_charger_state(regmap, &val->intval);
> + break;
> + case POWER_SUPPLY_PROP_CHARGE_TYPE:
> + ret = act8945a_get_charge_type(regmap, &val->intval);
> + break;
> + case POWER_SUPPLY_PROP_TECHNOLOGY:
> + val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
> + break;
> + case POWER_SUPPLY_PROP_HEALTH:
> + ret = act8945a_get_battery_health(charger,
> + regmap, &val->intval);
> + break;
> + case POWER_SUPPLY_PROP_MODEL_NAME:
> + val->strval = act8945a_charger_model;
> + break;
> + case POWER_SUPPLY_PROP_MANUFACTURER:
> + val->strval = act8945a_charger_manufacturer;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return ret;
> +}
> +
> +static const struct power_supply_desc act8945a_charger_desc = {
> + .name = "act8945a-charger",
> + .type = POWER_SUPPLY_TYPE_BATTERY,
> + .get_property = act8945a_charger_get_property,
> + .properties = act8945a_charger_props,
> + .num_properties = ARRAY_SIZE(act8945a_charger_props),
> +};
> +
> +#define DEFAULT_TOTAL_TIME_OUT 3
> +#define DEFAULT_PRE_TIME_OUT 40
> +#define DEFAULT_INPUT_OVP_THRESHOLD 6600
> +
> +static int act8945a_charger_parse_dt(struct device *dev,
> + struct act8945a_charger *charger)
> +{
> + struct device_node *np = dev->of_node;
> + enum of_gpio_flags flags;
> +
> + if (!np) {
> + dev_err(dev, "no charger of node\n");
> + return -EINVAL;
> + }
> +
> + charger->chglev_pin = of_get_named_gpio_flags(np,
> + "active-semi,chglev-gpio", 0, &flags);
> +
> + charger->chglev_value = (flags == OF_GPIO_ACTIVE_LOW) ? 0 : 1;
> +
> + charger->battery_temperature = !!of_get_property(np,
> + "active-semi,battery_temperature", NULL);
Just: of_property_read_bool()?
> +
> + if (of_property_read_u32(np, "active-semi,input_voltage_threshold",
> + &charger->input_voltage_threshold))
> + charger->input_voltage_threshold = DEFAULT_PRE_TIME_OUT;
> +
> + if (of_property_read_u32(np, "active-semi,precondition_timeout",
> + &charger->pre_time_out))
> + charger->pre_time_out = DEFAULT_PRE_TIME_OUT;
> +
> + if (of_property_read_u32(np, "active-semi,total_timeout",
> + &charger->tolal_time_out))
> + charger->tolal_time_out = DEFAULT_TOTAL_TIME_OUT;
> +
> + return 0;
> +}
> +
> +static int act8945a_charger_config(struct act8945a_charger *charger)
> +{
> + struct regmap *regmap = charger->act8945a_dev->regmap;
> + u8 value = 0;
> +
> + if (gpio_is_valid(charger->chglev_pin))
> + gpio_set_value(charger->chglev_pin, charger->chglev_value);
> +
> + switch (charger->input_voltage_threshold) {
> + case 8000:
> + value |= APCH_CFG_OVPSET_8V;
> + break;
> + case 7500:
> + value |= APCH_CFG_OVPSET_7V5;
> + break;
> + case 7000:
> + value |= APCH_CFG_OVPSET_7V;
> + break;
> + case 6600:
> + default:
> + value |= APCH_CFG_OVPSET_6V6;
> + break;
> + }
> +
> + switch (charger->pre_time_out) {
> + case 60:
> + value |= APCH_CFG_PRETIMO_60_MIN;
> + break;
> + case 80:
> + value |= APCH_CFG_PRETIMO_80_MIN;
> + break;
> + case 0:
> + value |= APCH_CFG_PRETIMO_DISABLED;
> + break;
> + case 40:
> + default:
> + value |= APCH_CFG_PRETIMO_40_MIN;
> + break;
> + }
> +
> + switch (charger->tolal_time_out) {
> + case 4:
> + value |= APCH_CFG_TOTTIMO_4_HOUR;
> + break;
> + case 5:
> + value |= APCH_CFG_TOTTIMO_5_HOUR;
> + break;
> + case 0:
> + value |= APCH_CFG_TOTTIMO_DISABLED;
> + break;
> + case 3:
> + default:
> + value |= APCH_CFG_TOTTIMO_3_HOUR;
> + break;
> + }
> +
> + return regmap_write(regmap, ACT8945A_APCH_CFG, value);
> +}
> +
> +static int act8945a_charger_probe(struct platform_device *pdev)
> +{
> + struct act8945a_dev *act8945a_dev = dev_get_drvdata(pdev->dev.parent);
> + struct act8945a_charger *charger;
> + struct power_supply_config psy_cfg = {};
> + int ret;
> +
> + charger = devm_kzalloc(&pdev->dev, sizeof(*charger), GFP_KERNEL);
> + if (!charger)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, charger);
> +
> + charger->dev = &pdev->dev;
> + charger->act8945a_dev = act8945a_dev;
> +
> + ret = act8945a_charger_parse_dt(&pdev->dev, charger);
> + if (ret)
> + return ret;
> +
> + ret = act8945a_charger_config(charger);
> + if (ret)
> + return ret;
> +
> + psy_cfg.of_node = pdev->dev.of_node;
> + psy_cfg.drv_data = charger;
> +
> + charger->psy = power_supply_register(&pdev->dev,
> + &act8945a_charger_desc,
> + &psy_cfg);
You can use devm-like() API for this and get rid of remove function...
but it is not a necessity I think. Up to you.
> + if (IS_ERR(charger->psy)) {
> + dev_err(act8945a_dev->dev, "failed to register power supply\n");
> + return PTR_ERR(charger->psy);
> + }
> +
> + dev_info(act8945a_dev->dev, "charger driver registered\n");
> +
> + return 0;
> +}
> +
> +static int act8945a_charger_remove(struct platform_device *pdev)
> +{
> + struct act8945a_charger *charger = platform_get_drvdata(pdev);
> +
> + power_supply_unregister(charger->psy);
> +
> + return 0;
> +}
> +
> +static struct platform_driver act8945a_charger_driver = {
> + .driver = {
> + .name = "act8945a-charger",
> + },
> + .probe = act8945a_charger_probe,
> + .remove = act8945a_charger_remove,
> +};
> +
> +static int __init act8945a_charger_init(void)
> +{
> + return platform_driver_register(&act8945a_charger_driver);
> +}
> +subsys_initcall(act8945a_charger_init);
Why subsys_initcall? This should be regular module_platform_driver.
> +
> +static void __exit act8945a_charger_cleanup(void)
> +{
> + platform_driver_unregister(&act8945a_charger_driver);
> +}
> +module_exit(act8945a_charger_cleanup);
> +
> +MODULE_DESCRIPTION("Active-semi ACT8945A ActivePath charger driver");
> +MODULE_AUTHOR("Wenyou Yang <wenyou.yang@atmel.com>");
> +MODULE_LICENSE("GPL v2");
In the comment at beginning you mentioned GPL v2+ so this should be "GPL".
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
@ 2016-01-08 3:55 ` Krzysztof Kozlowski
0 siblings, 0 replies; 29+ messages in thread
From: Krzysztof Kozlowski @ 2016-01-08 3:55 UTC (permalink / raw)
To: linux-arm-kernel
2016-01-08 11:13 GMT+09:00 Wenyou Yang <wenyou.yang@atmel.com>:
> This patch adds new driver for Active-semi ACT8945A ActivePath
> charger (part of ACT8945A MFD driver) providing power supply class
> information to userspace.
>
> The driver is configured through DTS (battery and system related
> settings) and sysfs entries (timers and input over-voltage threshold).
>
> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> ---
>
> drivers/power/Kconfig | 7 +
> drivers/power/Makefile | 1 +
> drivers/power/act8945a_charger.c | 397 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 405 insertions(+)
> create mode 100644 drivers/power/act8945a_charger.c
>
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 1ddd13c..ae75211 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -75,6 +75,13 @@ config BATTERY_88PM860X
> help
> Say Y here to enable battery monitor for Marvell 88PM860x chip.
>
> +config BATTERY_ACT8945A
> + tristate "Active-semi ACT8945A charger driver"
> + depends on MFD_ACT8945A
> + help
> + Say Y here to enable support for power supply provided by
> + Active-semi ActivePath ACT8945A charger.
> +
> config BATTERY_DS2760
> tristate "DS2760 battery driver (HP iPAQ & others)"
> depends on W1 && W1_SLAVE_DS2760
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index 0e4eab5..e46b75d 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -17,6 +17,7 @@ obj-$(CONFIG_WM8350_POWER) += wm8350_power.o
> obj-$(CONFIG_TEST_POWER) += test_power.o
>
> obj-$(CONFIG_BATTERY_88PM860X) += 88pm860x_battery.o
> +obj-$(CONFIG_BATTERY_ACT8945A) += act8945a_charger.o
> obj-$(CONFIG_BATTERY_DS2760) += ds2760_battery.o
> obj-$(CONFIG_BATTERY_DS2780) += ds2780_battery.o
> obj-$(CONFIG_BATTERY_DS2781) += ds2781_battery.o
> diff --git a/drivers/power/act8945a_charger.c b/drivers/power/act8945a_charger.c
> new file mode 100644
> index 0000000..5b55cd7
> --- /dev/null
> +++ b/drivers/power/act8945a_charger.c
> @@ -0,0 +1,397 @@
> +/*
> + * Power supply driver for the Active-semi ACT8945A PMIC
> + *
> + * Copyright (C) 2015 Atmel Corporation
> + * Wenyou Yang <wenyou.yang@atmel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/mfd/act8945a.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +#include <linux/regmap.h>
> +
> +static const char *act8945a_charger_model = "ACT8945A";
> +static const char *act8945a_charger_manufacturer = "Active-semi";
> +
> +/**
> + * ACT8945A Charger Register Map
> + */
> +
> +/* 0x70: Reserved */
> +#define ACT8945A_APCH_CFG 0x71
> +#define ACT8945A_APCH_STATUS 0x78
> +#define ACT8945A_APCH_CTRL 0x79
> +#define ACT8945A_APCH_STATE 0x7A
> +
> +/* ACT8945A_APCH_CFG */
> +#define APCH_CFG_OVPSET (0x03 << 0)
> +#define APCH_CFG_OVPSET_6V6 (0x0 << 0)
> +#define APCH_CFG_OVPSET_7V (0x1 << 0)
> +#define APCH_CFG_OVPSET_7V5 (0x2 << 0)
> +#define APCH_CFG_OVPSET_8V (0x3 << 0)
> +#define APCH_CFG_PRETIMO (0x03 << 2)
> +#define APCH_CFG_PRETIMO_40_MIN (0x0 << 2)
> +#define APCH_CFG_PRETIMO_60_MIN (0x1 << 2)
> +#define APCH_CFG_PRETIMO_80_MIN (0x2 << 2)
> +#define APCH_CFG_PRETIMO_DISABLED (0x3 << 2)
> +#define APCH_CFG_TOTTIMO (0x03 << 4)
> +#define APCH_CFG_TOTTIMO_3_HOUR (0x0 << 4)
> +#define APCH_CFG_TOTTIMO_4_HOUR (0x1 << 4)
> +#define APCH_CFG_TOTTIMO_5_HOUR (0x2 << 4)
> +#define APCH_CFG_TOTTIMO_DISABLED (0x3 << 4)
> +#define APCH_CFG_SUSCHG (0x01 << 7)
> +
> +#define APCH_STATUS_CHGDAT (0x01 << 0)
> +#define APCH_STATUS_INDAT (0x01 << 1)
> +#define APCH_STATUS_TEMPDAT (0x01 << 2)
> +#define APCH_STATUS_TIMRDAT (0x01 << 3)
> +#define APCH_STATUS_CHGSTAT (0x01 << 4)
> +#define APCH_STATUS_INSTAT (0x01 << 5)
> +#define APCH_STATUS_TEMPSTAT (0x01 << 6)
> +#define APCH_STATUS_TIMRSTAT (0x01 << 7)
> +
> +#define APCH_CTRL_CHGEOCOUT (0x01 << 0)
> +#define APCH_CTRL_INDIS (0x01 << 1)
> +#define APCH_CTRL_TEMPOUT (0x01 << 2)
> +#define APCH_CTRL_TIMRPRE (0x01 << 3)
> +#define APCH_CTRL_CHGEOCIN (0x01 << 4)
> +#define APCH_CTRL_INCON (0x01 << 5)
> +#define APCH_CTRL_TEMPIN (0x01 << 6)
> +#define APCH_CTRL_TIMRTOT (0x01 << 7)
> +
> +#define APCH_STATE_ACINSTAT (0x01 << 1)
> +#define APCH_STATE_CSTATE (0x03 << 4)
> +#define APCH_STATE_CSTATE_SHIFT 4
> +#define APCH_STATE_CSTATE_DISABLED 0x00
> +#define APCH_STATE_CSTATE_EOC 0x01
> +#define APCH_STATE_CSTATE_FAST 0x02
> +#define APCH_STATE_CSTATE_PRE 0x03
> +
> +struct act8945a_charger {
> + struct device *dev;
> + struct act8945a_dev *act8945a_dev;
> + struct power_supply *psy;
> +
> + u32 tolal_time_out;
> + u32 pre_time_out;
> + u32 input_voltage_threshold;
> + bool battery_temperature;
> + int chglev_pin;
> + int chglev_value;
> +};
> +
> +static int act8945a_get_charger_state(struct regmap *regmap, int *val)
> +{
> + int ret;
> + unsigned int status, state;
> +
> + ret = regmap_read(regmap, ACT8945A_APCH_STATUS, &status);
> + if (ret < 0)
> + return ret;
> +
> + ret = regmap_read(regmap, ACT8945A_APCH_STATE, &state);
> + if (ret < 0)
> + return ret;
> +
> + state &= APCH_STATE_CSTATE;
> + state >>= APCH_STATE_CSTATE_SHIFT;
> +
> + if (state == APCH_STATE_CSTATE_EOC) {
> + if (status & APCH_STATUS_CHGDAT)
> + *val = POWER_SUPPLY_STATUS_FULL;
> + else
> + *val = POWER_SUPPLY_STATUS_NOT_CHARGING;
> + } else if ((state == APCH_STATE_CSTATE_FAST) ||
> + (state == APCH_STATE_CSTATE_PRE)) {
> + *val = POWER_SUPPLY_STATUS_CHARGING;
> + } else {
> + *val = POWER_SUPPLY_STATUS_NOT_CHARGING;
> + }
> +
> + return 0;
> +}
> +
> +static int act8945a_get_charge_type(struct regmap *regmap, int *val)
> +{
> + int ret;
> + unsigned int state;
> +
> + ret = regmap_read(regmap, ACT8945A_APCH_STATE, &state);
> + if (ret < 0)
> + return ret;
> +
> + state &= APCH_STATE_CSTATE;
> + state >>= APCH_STATE_CSTATE_SHIFT;
> +
> + switch (state) {
> + case APCH_STATE_CSTATE_PRE:
> + *val = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
> + break;
> + case APCH_STATE_CSTATE_FAST:
> + *val = POWER_SUPPLY_CHARGE_TYPE_FAST;
> + break;
> + case APCH_STATE_CSTATE_EOC:
> + case APCH_STATE_CSTATE_DISABLED:
> + default:
> + *val = POWER_SUPPLY_CHARGE_TYPE_NONE;
> + }
> +
> + return 0;
> +}
> +
> +static int act8945a_get_battery_health(struct act8945a_charger *charger,
> + struct regmap *regmap, int *val)
> +{
> + int ret;
> + unsigned int status;
> +
> + ret = regmap_read(regmap, ACT8945A_APCH_STATUS, &status);
> + if (ret < 0)
> + return ret;
> +
> + if (charger->battery_temperature && !(status & APCH_STATUS_TEMPDAT))
> + *val = POWER_SUPPLY_HEALTH_OVERHEAT;
> + else if (!(status & APCH_STATUS_INDAT))
> + *val = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
> + else if (status & APCH_STATUS_TIMRDAT)
> + *val = POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE;
> + else
> + *val = POWER_SUPPLY_HEALTH_GOOD;
> +
> + return 0;
> +}
> +
> +static enum power_supply_property act8945a_charger_props[] = {
> + POWER_SUPPLY_PROP_STATUS,
> + POWER_SUPPLY_PROP_CHARGE_TYPE,
> + POWER_SUPPLY_PROP_TECHNOLOGY,
> + POWER_SUPPLY_PROP_HEALTH,
> + POWER_SUPPLY_PROP_MODEL_NAME,
> + POWER_SUPPLY_PROP_MANUFACTURER
> +};
> +
> +static int act8945a_charger_get_property(struct power_supply *psy,
> + enum power_supply_property prop,
> + union power_supply_propval *val)
> +{
> + struct act8945a_charger *charger = power_supply_get_drvdata(psy);
> + struct regmap *regmap = charger->act8945a_dev->regmap;
> + int ret = 0;
> +
> + switch (prop) {
> + case POWER_SUPPLY_PROP_STATUS:
> + ret = act8945a_get_charger_state(regmap, &val->intval);
> + break;
> + case POWER_SUPPLY_PROP_CHARGE_TYPE:
> + ret = act8945a_get_charge_type(regmap, &val->intval);
> + break;
> + case POWER_SUPPLY_PROP_TECHNOLOGY:
> + val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
> + break;
> + case POWER_SUPPLY_PROP_HEALTH:
> + ret = act8945a_get_battery_health(charger,
> + regmap, &val->intval);
> + break;
> + case POWER_SUPPLY_PROP_MODEL_NAME:
> + val->strval = act8945a_charger_model;
> + break;
> + case POWER_SUPPLY_PROP_MANUFACTURER:
> + val->strval = act8945a_charger_manufacturer;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return ret;
> +}
> +
> +static const struct power_supply_desc act8945a_charger_desc = {
> + .name = "act8945a-charger",
> + .type = POWER_SUPPLY_TYPE_BATTERY,
> + .get_property = act8945a_charger_get_property,
> + .properties = act8945a_charger_props,
> + .num_properties = ARRAY_SIZE(act8945a_charger_props),
> +};
> +
> +#define DEFAULT_TOTAL_TIME_OUT 3
> +#define DEFAULT_PRE_TIME_OUT 40
> +#define DEFAULT_INPUT_OVP_THRESHOLD 6600
> +
> +static int act8945a_charger_parse_dt(struct device *dev,
> + struct act8945a_charger *charger)
> +{
> + struct device_node *np = dev->of_node;
> + enum of_gpio_flags flags;
> +
> + if (!np) {
> + dev_err(dev, "no charger of node\n");
> + return -EINVAL;
> + }
> +
> + charger->chglev_pin = of_get_named_gpio_flags(np,
> + "active-semi,chglev-gpio", 0, &flags);
> +
> + charger->chglev_value = (flags == OF_GPIO_ACTIVE_LOW) ? 0 : 1;
> +
> + charger->battery_temperature = !!of_get_property(np,
> + "active-semi,battery_temperature", NULL);
Just: of_property_read_bool()?
> +
> + if (of_property_read_u32(np, "active-semi,input_voltage_threshold",
> + &charger->input_voltage_threshold))
> + charger->input_voltage_threshold = DEFAULT_PRE_TIME_OUT;
> +
> + if (of_property_read_u32(np, "active-semi,precondition_timeout",
> + &charger->pre_time_out))
> + charger->pre_time_out = DEFAULT_PRE_TIME_OUT;
> +
> + if (of_property_read_u32(np, "active-semi,total_timeout",
> + &charger->tolal_time_out))
> + charger->tolal_time_out = DEFAULT_TOTAL_TIME_OUT;
> +
> + return 0;
> +}
> +
> +static int act8945a_charger_config(struct act8945a_charger *charger)
> +{
> + struct regmap *regmap = charger->act8945a_dev->regmap;
> + u8 value = 0;
> +
> + if (gpio_is_valid(charger->chglev_pin))
> + gpio_set_value(charger->chglev_pin, charger->chglev_value);
> +
> + switch (charger->input_voltage_threshold) {
> + case 8000:
> + value |= APCH_CFG_OVPSET_8V;
> + break;
> + case 7500:
> + value |= APCH_CFG_OVPSET_7V5;
> + break;
> + case 7000:
> + value |= APCH_CFG_OVPSET_7V;
> + break;
> + case 6600:
> + default:
> + value |= APCH_CFG_OVPSET_6V6;
> + break;
> + }
> +
> + switch (charger->pre_time_out) {
> + case 60:
> + value |= APCH_CFG_PRETIMO_60_MIN;
> + break;
> + case 80:
> + value |= APCH_CFG_PRETIMO_80_MIN;
> + break;
> + case 0:
> + value |= APCH_CFG_PRETIMO_DISABLED;
> + break;
> + case 40:
> + default:
> + value |= APCH_CFG_PRETIMO_40_MIN;
> + break;
> + }
> +
> + switch (charger->tolal_time_out) {
> + case 4:
> + value |= APCH_CFG_TOTTIMO_4_HOUR;
> + break;
> + case 5:
> + value |= APCH_CFG_TOTTIMO_5_HOUR;
> + break;
> + case 0:
> + value |= APCH_CFG_TOTTIMO_DISABLED;
> + break;
> + case 3:
> + default:
> + value |= APCH_CFG_TOTTIMO_3_HOUR;
> + break;
> + }
> +
> + return regmap_write(regmap, ACT8945A_APCH_CFG, value);
> +}
> +
> +static int act8945a_charger_probe(struct platform_device *pdev)
> +{
> + struct act8945a_dev *act8945a_dev = dev_get_drvdata(pdev->dev.parent);
> + struct act8945a_charger *charger;
> + struct power_supply_config psy_cfg = {};
> + int ret;
> +
> + charger = devm_kzalloc(&pdev->dev, sizeof(*charger), GFP_KERNEL);
> + if (!charger)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, charger);
> +
> + charger->dev = &pdev->dev;
> + charger->act8945a_dev = act8945a_dev;
> +
> + ret = act8945a_charger_parse_dt(&pdev->dev, charger);
> + if (ret)
> + return ret;
> +
> + ret = act8945a_charger_config(charger);
> + if (ret)
> + return ret;
> +
> + psy_cfg.of_node = pdev->dev.of_node;
> + psy_cfg.drv_data = charger;
> +
> + charger->psy = power_supply_register(&pdev->dev,
> + &act8945a_charger_desc,
> + &psy_cfg);
You can use devm-like() API for this and get rid of remove function...
but it is not a necessity I think. Up to you.
> + if (IS_ERR(charger->psy)) {
> + dev_err(act8945a_dev->dev, "failed to register power supply\n");
> + return PTR_ERR(charger->psy);
> + }
> +
> + dev_info(act8945a_dev->dev, "charger driver registered\n");
> +
> + return 0;
> +}
> +
> +static int act8945a_charger_remove(struct platform_device *pdev)
> +{
> + struct act8945a_charger *charger = platform_get_drvdata(pdev);
> +
> + power_supply_unregister(charger->psy);
> +
> + return 0;
> +}
> +
> +static struct platform_driver act8945a_charger_driver = {
> + .driver = {
> + .name = "act8945a-charger",
> + },
> + .probe = act8945a_charger_probe,
> + .remove = act8945a_charger_remove,
> +};
> +
> +static int __init act8945a_charger_init(void)
> +{
> + return platform_driver_register(&act8945a_charger_driver);
> +}
> +subsys_initcall(act8945a_charger_init);
Why subsys_initcall? This should be regular module_platform_driver.
> +
> +static void __exit act8945a_charger_cleanup(void)
> +{
> + platform_driver_unregister(&act8945a_charger_driver);
> +}
> +module_exit(act8945a_charger_cleanup);
> +
> +MODULE_DESCRIPTION("Active-semi ACT8945A ActivePath charger driver");
> +MODULE_AUTHOR("Wenyou Yang <wenyou.yang@atmel.com>");
> +MODULE_LICENSE("GPL v2");
In the comment at beginning you mentioned GPL v2+ so this should be "GPL".
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 29+ messages in thread* RE: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
2016-01-08 3:55 ` Krzysztof Kozlowski
(?)
@ 2016-01-08 6:43 ` Yang, Wenyou
-1 siblings, 0 replies; 29+ messages in thread
From: Yang, Wenyou @ 2016-01-08 6:43 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Ferre, Nicolas, linux-arm-kernel, linux-kernel, linux-pm
Hello Krzysztof,
Thank you for your commends.
They will be fixed in the next version.
> -----Original Message-----
> From: k.kozlowski.k@gmail.com [mailto:k.kozlowski.k@gmail.com] On Behalf Of
> Krzysztof Kozlowski
> Sent: 2016年1月8日 11:55
> To: Yang, Wenyou <Wenyou.Yang@atmel.com>
> Cc: Sebastian Reichel <sre@kernel.org>; Dmitry Eremin-Solenikov
> <dbaryshkov@gmail.com>; David Woodhouse <dwmw2@infradead.org>; Rob
> Herring <robh+dt@kernel.org>; Pawel Moll <pawel.moll@arm.com>; Mark
> Rutland <mark.rutland@arm.com>; Ian Campbell <ijc+devicetree@hellion.org.uk>;
> Kumar Gala <galak@codeaurora.org>; Ferre, Nicolas
> <Nicolas.FERRE@atmel.com>; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; linux-pm@vger.kernel.org
> Subject: Re: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
>
> 2016-01-08 11:13 GMT+09:00 Wenyou Yang <wenyou.yang@atmel.com>:
> > This patch adds new driver for Active-semi ACT8945A ActivePath charger
> > (part of ACT8945A MFD driver) providing power supply class information
> > to userspace.
> >
> > The driver is configured through DTS (battery and system related
> > settings) and sysfs entries (timers and input over-voltage threshold).
> >
> > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> > ---
> >
> > drivers/power/Kconfig | 7 +
> > drivers/power/Makefile | 1 +
> > drivers/power/act8945a_charger.c | 397
> > ++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 405 insertions(+)
> > create mode 100644 drivers/power/act8945a_charger.c
> >
> > diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig index
> > 1ddd13c..ae75211 100644
> > --- a/drivers/power/Kconfig
> > +++ b/drivers/power/Kconfig
> > @@ -75,6 +75,13 @@ config BATTERY_88PM860X
> > help
> > Say Y here to enable battery monitor for Marvell 88PM860x chip.
> >
> > +config BATTERY_ACT8945A
> > + tristate "Active-semi ACT8945A charger driver"
> > + depends on MFD_ACT8945A
> > + help
> > + Say Y here to enable support for power supply provided by
> > + Active-semi ActivePath ACT8945A charger.
> > +
> > config BATTERY_DS2760
> > tristate "DS2760 battery driver (HP iPAQ & others)"
> > depends on W1 && W1_SLAVE_DS2760 diff --git
> > a/drivers/power/Makefile b/drivers/power/Makefile index
> > 0e4eab5..e46b75d 100644
> > --- a/drivers/power/Makefile
> > +++ b/drivers/power/Makefile
> > @@ -17,6 +17,7 @@ obj-$(CONFIG_WM8350_POWER) +=
> wm8350_power.o
> > obj-$(CONFIG_TEST_POWER) += test_power.o
> >
> > obj-$(CONFIG_BATTERY_88PM860X) += 88pm860x_battery.o
> > +obj-$(CONFIG_BATTERY_ACT8945A) += act8945a_charger.o
> > obj-$(CONFIG_BATTERY_DS2760) += ds2760_battery.o
> > obj-$(CONFIG_BATTERY_DS2780) += ds2780_battery.o
> > obj-$(CONFIG_BATTERY_DS2781) += ds2781_battery.o
> > diff --git a/drivers/power/act8945a_charger.c
> > b/drivers/power/act8945a_charger.c
> > new file mode 100644
> > index 0000000..5b55cd7
> > --- /dev/null
> > +++ b/drivers/power/act8945a_charger.c
> > @@ -0,0 +1,397 @@
> > +/*
> > + * Power supply driver for the Active-semi ACT8945A PMIC
> > + *
> > + * Copyright (C) 2015 Atmel Corporation
> > + * Wenyou Yang <wenyou.yang@atmel.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > +modify
> > + * it under the terms of the GNU General Public License as published
> > +by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
> the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/mfd/act8945a.h>
> > +#include <linux/of.h>
> > +#include <linux/of_gpio.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/power_supply.h>
> > +#include <linux/regmap.h>
> > +
> > +static const char *act8945a_charger_model = "ACT8945A"; static const
> > +char *act8945a_charger_manufacturer = "Active-semi";
> > +
> > +/**
> > + * ACT8945A Charger Register Map
> > + */
> > +
> > +/* 0x70: Reserved */
> > +#define ACT8945A_APCH_CFG 0x71
> > +#define ACT8945A_APCH_STATUS 0x78
> > +#define ACT8945A_APCH_CTRL 0x79
> > +#define ACT8945A_APCH_STATE 0x7A
> > +
> > +/* ACT8945A_APCH_CFG */
> > +#define APCH_CFG_OVPSET (0x03 << 0)
> > +#define APCH_CFG_OVPSET_6V6 (0x0 << 0)
> > +#define APCH_CFG_OVPSET_7V (0x1 << 0)
> > +#define APCH_CFG_OVPSET_7V5 (0x2 << 0)
> > +#define APCH_CFG_OVPSET_8V (0x3 << 0)
> > +#define APCH_CFG_PRETIMO (0x03 << 2)
> > +#define APCH_CFG_PRETIMO_40_MIN (0x0 << 2)
> > +#define APCH_CFG_PRETIMO_60_MIN (0x1 << 2)
> > +#define APCH_CFG_PRETIMO_80_MIN (0x2 << 2)
> > +#define APCH_CFG_PRETIMO_DISABLED (0x3 << 2)
> > +#define APCH_CFG_TOTTIMO (0x03 << 4)
> > +#define APCH_CFG_TOTTIMO_3_HOUR (0x0 << 4)
> > +#define APCH_CFG_TOTTIMO_4_HOUR (0x1 << 4)
> > +#define APCH_CFG_TOTTIMO_5_HOUR (0x2 << 4)
> > +#define APCH_CFG_TOTTIMO_DISABLED (0x3 << 4)
> > +#define APCH_CFG_SUSCHG (0x01 << 7)
> > +
> > +#define APCH_STATUS_CHGDAT (0x01 << 0)
> > +#define APCH_STATUS_INDAT (0x01 << 1)
> > +#define APCH_STATUS_TEMPDAT (0x01 << 2)
> > +#define APCH_STATUS_TIMRDAT (0x01 << 3)
> > +#define APCH_STATUS_CHGSTAT (0x01 << 4)
> > +#define APCH_STATUS_INSTAT (0x01 << 5)
> > +#define APCH_STATUS_TEMPSTAT (0x01 << 6)
> > +#define APCH_STATUS_TIMRSTAT (0x01 << 7)
> > +
> > +#define APCH_CTRL_CHGEOCOUT (0x01 << 0)
> > +#define APCH_CTRL_INDIS (0x01 << 1)
> > +#define APCH_CTRL_TEMPOUT (0x01 << 2)
> > +#define APCH_CTRL_TIMRPRE (0x01 << 3)
> > +#define APCH_CTRL_CHGEOCIN (0x01 << 4)
> > +#define APCH_CTRL_INCON (0x01 << 5)
> > +#define APCH_CTRL_TEMPIN (0x01 << 6)
> > +#define APCH_CTRL_TIMRTOT (0x01 << 7)
> > +
> > +#define APCH_STATE_ACINSTAT (0x01 << 1)
> > +#define APCH_STATE_CSTATE (0x03 << 4)
> > +#define APCH_STATE_CSTATE_SHIFT 4
> > +#define APCH_STATE_CSTATE_DISABLED 0x00
> > +#define APCH_STATE_CSTATE_EOC 0x01
> > +#define APCH_STATE_CSTATE_FAST 0x02
> > +#define APCH_STATE_CSTATE_PRE 0x03
> > +
> > +struct act8945a_charger {
> > + struct device *dev;
> > + struct act8945a_dev *act8945a_dev;
> > + struct power_supply *psy;
> > +
> > + u32 tolal_time_out;
> > + u32 pre_time_out;
> > + u32 input_voltage_threshold;
> > + bool battery_temperature;
> > + int chglev_pin;
> > + int chglev_value;
> > +};
> > +
> > +static int act8945a_get_charger_state(struct regmap *regmap, int
> > +*val) {
> > + int ret;
> > + unsigned int status, state;
> > +
> > + ret = regmap_read(regmap, ACT8945A_APCH_STATUS, &status);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = regmap_read(regmap, ACT8945A_APCH_STATE, &state);
> > + if (ret < 0)
> > + return ret;
> > +
> > + state &= APCH_STATE_CSTATE;
> > + state >>= APCH_STATE_CSTATE_SHIFT;
> > +
> > + if (state == APCH_STATE_CSTATE_EOC) {
> > + if (status & APCH_STATUS_CHGDAT)
> > + *val = POWER_SUPPLY_STATUS_FULL;
> > + else
> > + *val = POWER_SUPPLY_STATUS_NOT_CHARGING;
> > + } else if ((state == APCH_STATE_CSTATE_FAST) ||
> > + (state == APCH_STATE_CSTATE_PRE)) {
> > + *val = POWER_SUPPLY_STATUS_CHARGING;
> > + } else {
> > + *val = POWER_SUPPLY_STATUS_NOT_CHARGING;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int act8945a_get_charge_type(struct regmap *regmap, int *val)
> > +{
> > + int ret;
> > + unsigned int state;
> > +
> > + ret = regmap_read(regmap, ACT8945A_APCH_STATE, &state);
> > + if (ret < 0)
> > + return ret;
> > +
> > + state &= APCH_STATE_CSTATE;
> > + state >>= APCH_STATE_CSTATE_SHIFT;
> > +
> > + switch (state) {
> > + case APCH_STATE_CSTATE_PRE:
> > + *val = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
> > + break;
> > + case APCH_STATE_CSTATE_FAST:
> > + *val = POWER_SUPPLY_CHARGE_TYPE_FAST;
> > + break;
> > + case APCH_STATE_CSTATE_EOC:
> > + case APCH_STATE_CSTATE_DISABLED:
> > + default:
> > + *val = POWER_SUPPLY_CHARGE_TYPE_NONE;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int act8945a_get_battery_health(struct act8945a_charger *charger,
> > + struct regmap *regmap, int *val)
> > +{
> > + int ret;
> > + unsigned int status;
> > +
> > + ret = regmap_read(regmap, ACT8945A_APCH_STATUS, &status);
> > + if (ret < 0)
> > + return ret;
> > +
> > + if (charger->battery_temperature && !(status &
> APCH_STATUS_TEMPDAT))
> > + *val = POWER_SUPPLY_HEALTH_OVERHEAT;
> > + else if (!(status & APCH_STATUS_INDAT))
> > + *val = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
> > + else if (status & APCH_STATUS_TIMRDAT)
> > + *val = POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE;
> > + else
> > + *val = POWER_SUPPLY_HEALTH_GOOD;
> > +
> > + return 0;
> > +}
> > +
> > +static enum power_supply_property act8945a_charger_props[] = {
> > + POWER_SUPPLY_PROP_STATUS,
> > + POWER_SUPPLY_PROP_CHARGE_TYPE,
> > + POWER_SUPPLY_PROP_TECHNOLOGY,
> > + POWER_SUPPLY_PROP_HEALTH,
> > + POWER_SUPPLY_PROP_MODEL_NAME,
> > + POWER_SUPPLY_PROP_MANUFACTURER };
> > +
> > +static int act8945a_charger_get_property(struct power_supply *psy,
> > + enum power_supply_property prop,
> > + union power_supply_propval *val) {
> > + struct act8945a_charger *charger = power_supply_get_drvdata(psy);
> > + struct regmap *regmap = charger->act8945a_dev->regmap;
> > + int ret = 0;
> > +
> > + switch (prop) {
> > + case POWER_SUPPLY_PROP_STATUS:
> > + ret = act8945a_get_charger_state(regmap, &val->intval);
> > + break;
> > + case POWER_SUPPLY_PROP_CHARGE_TYPE:
> > + ret = act8945a_get_charge_type(regmap, &val->intval);
> > + break;
> > + case POWER_SUPPLY_PROP_TECHNOLOGY:
> > + val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
> > + break;
> > + case POWER_SUPPLY_PROP_HEALTH:
> > + ret = act8945a_get_battery_health(charger,
> > + regmap, &val->intval);
> > + break;
> > + case POWER_SUPPLY_PROP_MODEL_NAME:
> > + val->strval = act8945a_charger_model;
> > + break;
> > + case POWER_SUPPLY_PROP_MANUFACTURER:
> > + val->strval = act8945a_charger_manufacturer;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static const struct power_supply_desc act8945a_charger_desc = {
> > + .name = "act8945a-charger",
> > + .type = POWER_SUPPLY_TYPE_BATTERY,
> > + .get_property = act8945a_charger_get_property,
> > + .properties = act8945a_charger_props,
> > + .num_properties = ARRAY_SIZE(act8945a_charger_props),
> > +};
> > +
> > +#define DEFAULT_TOTAL_TIME_OUT 3
> > +#define DEFAULT_PRE_TIME_OUT 40
> > +#define DEFAULT_INPUT_OVP_THRESHOLD 6600
> > +
> > +static int act8945a_charger_parse_dt(struct device *dev,
> > + struct act8945a_charger *charger)
> > +{
> > + struct device_node *np = dev->of_node;
> > + enum of_gpio_flags flags;
> > +
> > + if (!np) {
> > + dev_err(dev, "no charger of node\n");
> > + return -EINVAL;
> > + }
> > +
> > + charger->chglev_pin = of_get_named_gpio_flags(np,
> > + "active-semi,chglev-gpio", 0, &flags);
> > +
> > + charger->chglev_value = (flags == OF_GPIO_ACTIVE_LOW) ? 0 : 1;
> > +
> > + charger->battery_temperature = !!of_get_property(np,
> > + "active-semi,battery_temperature",
> > + NULL);
>
> Just: of_property_read_bool()?
>
> > +
> > + if (of_property_read_u32(np, "active-semi,input_voltage_threshold",
> > + &charger->input_voltage_threshold))
> > + charger->input_voltage_threshold =
> > + DEFAULT_PRE_TIME_OUT;
> > +
> > + if (of_property_read_u32(np, "active-semi,precondition_timeout",
> > + &charger->pre_time_out))
> > + charger->pre_time_out = DEFAULT_PRE_TIME_OUT;
> > +
> > + if (of_property_read_u32(np, "active-semi,total_timeout",
> > + &charger->tolal_time_out))
> > + charger->tolal_time_out = DEFAULT_TOTAL_TIME_OUT;
> > +
> > + return 0;
> > +}
> > +
> > +static int act8945a_charger_config(struct act8945a_charger *charger)
> > +{
> > + struct regmap *regmap = charger->act8945a_dev->regmap;
> > + u8 value = 0;
> > +
> > + if (gpio_is_valid(charger->chglev_pin))
> > + gpio_set_value(charger->chglev_pin,
> > + charger->chglev_value);
> > +
> > + switch (charger->input_voltage_threshold) {
> > + case 8000:
> > + value |= APCH_CFG_OVPSET_8V;
> > + break;
> > + case 7500:
> > + value |= APCH_CFG_OVPSET_7V5;
> > + break;
> > + case 7000:
> > + value |= APCH_CFG_OVPSET_7V;
> > + break;
> > + case 6600:
> > + default:
> > + value |= APCH_CFG_OVPSET_6V6;
> > + break;
> > + }
> > +
> > + switch (charger->pre_time_out) {
> > + case 60:
> > + value |= APCH_CFG_PRETIMO_60_MIN;
> > + break;
> > + case 80:
> > + value |= APCH_CFG_PRETIMO_80_MIN;
> > + break;
> > + case 0:
> > + value |= APCH_CFG_PRETIMO_DISABLED;
> > + break;
> > + case 40:
> > + default:
> > + value |= APCH_CFG_PRETIMO_40_MIN;
> > + break;
> > + }
> > +
> > + switch (charger->tolal_time_out) {
> > + case 4:
> > + value |= APCH_CFG_TOTTIMO_4_HOUR;
> > + break;
> > + case 5:
> > + value |= APCH_CFG_TOTTIMO_5_HOUR;
> > + break;
> > + case 0:
> > + value |= APCH_CFG_TOTTIMO_DISABLED;
> > + break;
> > + case 3:
> > + default:
> > + value |= APCH_CFG_TOTTIMO_3_HOUR;
> > + break;
> > + }
> > +
> > + return regmap_write(regmap, ACT8945A_APCH_CFG, value); }
> > +
> > +static int act8945a_charger_probe(struct platform_device *pdev) {
> > + struct act8945a_dev *act8945a_dev = dev_get_drvdata(pdev->dev.parent);
> > + struct act8945a_charger *charger;
> > + struct power_supply_config psy_cfg = {};
> > + int ret;
> > +
> > + charger = devm_kzalloc(&pdev->dev, sizeof(*charger), GFP_KERNEL);
> > + if (!charger)
> > + return -ENOMEM;
> > +
> > + platform_set_drvdata(pdev, charger);
> > +
> > + charger->dev = &pdev->dev;
> > + charger->act8945a_dev = act8945a_dev;
> > +
> > + ret = act8945a_charger_parse_dt(&pdev->dev, charger);
> > + if (ret)
> > + return ret;
> > +
> > + ret = act8945a_charger_config(charger);
> > + if (ret)
> > + return ret;
> > +
> > + psy_cfg.of_node = pdev->dev.of_node;
> > + psy_cfg.drv_data = charger;
> > +
> > + charger->psy = power_supply_register(&pdev->dev,
> > + &act8945a_charger_desc,
> > + &psy_cfg);
>
> You can use devm-like() API for this and get rid of remove function...
> but it is not a necessity I think. Up to you.
>
> > + if (IS_ERR(charger->psy)) {
> > + dev_err(act8945a_dev->dev, "failed to register power supply\n");
> > + return PTR_ERR(charger->psy);
> > + }
> > +
> > + dev_info(act8945a_dev->dev, "charger driver registered\n");
> > +
> > + return 0;
> > +}
> > +
> > +static int act8945a_charger_remove(struct platform_device *pdev) {
> > + struct act8945a_charger *charger = platform_get_drvdata(pdev);
> > +
> > + power_supply_unregister(charger->psy);
> > +
> > + return 0;
> > +}
> > +
> > +static struct platform_driver act8945a_charger_driver = {
> > + .driver = {
> > + .name = "act8945a-charger",
> > + },
> > + .probe = act8945a_charger_probe,
> > + .remove = act8945a_charger_remove, };
> > +
> > +static int __init act8945a_charger_init(void) {
> > + return platform_driver_register(&act8945a_charger_driver);
> > +}
> > +subsys_initcall(act8945a_charger_init);
>
> Why subsys_initcall? This should be regular module_platform_driver.
I want it to register early enough as MFD. Maybe I am wrong.
>
> > +
> > +static void __exit act8945a_charger_cleanup(void) {
> > + platform_driver_unregister(&act8945a_charger_driver);
> > +}
> > +module_exit(act8945a_charger_cleanup);
> > +
> > +MODULE_DESCRIPTION("Active-semi ACT8945A ActivePath charger driver");
> > +MODULE_AUTHOR("Wenyou Yang <wenyou.yang@atmel.com>");
> > +MODULE_LICENSE("GPL v2");
>
> In the comment at beginning you mentioned GPL v2+ so this should be "GPL".
>
> Best regards,
> Krzysztof
Best Regards,
Wenyou Yang
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
@ 2016-01-08 6:43 ` Yang, Wenyou
0 siblings, 0 replies; 29+ messages in thread
From: Yang, Wenyou @ 2016-01-08 6:43 UTC (permalink / raw)
To: linux-arm-kernel
Hello Krzysztof,
Thank you for your commends.
They will be fixed in the next version.
> -----Original Message-----
> From: k.kozlowski.k at gmail.com [mailto:k.kozlowski.k at gmail.com] On Behalf Of
> Krzysztof Kozlowski
> Sent: 2016?1?8? 11:55
> To: Yang, Wenyou <Wenyou.Yang@atmel.com>
> Cc: Sebastian Reichel <sre@kernel.org>; Dmitry Eremin-Solenikov
> <dbaryshkov@gmail.com>; David Woodhouse <dwmw2@infradead.org>; Rob
> Herring <robh+dt@kernel.org>; Pawel Moll <pawel.moll@arm.com>; Mark
> Rutland <mark.rutland@arm.com>; Ian Campbell <ijc+devicetree@hellion.org.uk>;
> Kumar Gala <galak@codeaurora.org>; Ferre, Nicolas
> <Nicolas.FERRE@atmel.com>; linux-arm-kernel at lists.infradead.org; linux-
> kernel at vger.kernel.org; linux-pm at vger.kernel.org
> Subject: Re: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
>
> 2016-01-08 11:13 GMT+09:00 Wenyou Yang <wenyou.yang@atmel.com>:
> > This patch adds new driver for Active-semi ACT8945A ActivePath charger
> > (part of ACT8945A MFD driver) providing power supply class information
> > to userspace.
> >
> > The driver is configured through DTS (battery and system related
> > settings) and sysfs entries (timers and input over-voltage threshold).
> >
> > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> > ---
> >
> > drivers/power/Kconfig | 7 +
> > drivers/power/Makefile | 1 +
> > drivers/power/act8945a_charger.c | 397
> > ++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 405 insertions(+)
> > create mode 100644 drivers/power/act8945a_charger.c
> >
> > diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig index
> > 1ddd13c..ae75211 100644
> > --- a/drivers/power/Kconfig
> > +++ b/drivers/power/Kconfig
> > @@ -75,6 +75,13 @@ config BATTERY_88PM860X
> > help
> > Say Y here to enable battery monitor for Marvell 88PM860x chip.
> >
> > +config BATTERY_ACT8945A
> > + tristate "Active-semi ACT8945A charger driver"
> > + depends on MFD_ACT8945A
> > + help
> > + Say Y here to enable support for power supply provided by
> > + Active-semi ActivePath ACT8945A charger.
> > +
> > config BATTERY_DS2760
> > tristate "DS2760 battery driver (HP iPAQ & others)"
> > depends on W1 && W1_SLAVE_DS2760 diff --git
> > a/drivers/power/Makefile b/drivers/power/Makefile index
> > 0e4eab5..e46b75d 100644
> > --- a/drivers/power/Makefile
> > +++ b/drivers/power/Makefile
> > @@ -17,6 +17,7 @@ obj-$(CONFIG_WM8350_POWER) +=
> wm8350_power.o
> > obj-$(CONFIG_TEST_POWER) += test_power.o
> >
> > obj-$(CONFIG_BATTERY_88PM860X) += 88pm860x_battery.o
> > +obj-$(CONFIG_BATTERY_ACT8945A) += act8945a_charger.o
> > obj-$(CONFIG_BATTERY_DS2760) += ds2760_battery.o
> > obj-$(CONFIG_BATTERY_DS2780) += ds2780_battery.o
> > obj-$(CONFIG_BATTERY_DS2781) += ds2781_battery.o
> > diff --git a/drivers/power/act8945a_charger.c
> > b/drivers/power/act8945a_charger.c
> > new file mode 100644
> > index 0000000..5b55cd7
> > --- /dev/null
> > +++ b/drivers/power/act8945a_charger.c
> > @@ -0,0 +1,397 @@
> > +/*
> > + * Power supply driver for the Active-semi ACT8945A PMIC
> > + *
> > + * Copyright (C) 2015 Atmel Corporation
> > + * Wenyou Yang <wenyou.yang@atmel.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > +modify
> > + * it under the terms of the GNU General Public License as published
> > +by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
> the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/mfd/act8945a.h>
> > +#include <linux/of.h>
> > +#include <linux/of_gpio.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/power_supply.h>
> > +#include <linux/regmap.h>
> > +
> > +static const char *act8945a_charger_model = "ACT8945A"; static const
> > +char *act8945a_charger_manufacturer = "Active-semi";
> > +
> > +/**
> > + * ACT8945A Charger Register Map
> > + */
> > +
> > +/* 0x70: Reserved */
> > +#define ACT8945A_APCH_CFG 0x71
> > +#define ACT8945A_APCH_STATUS 0x78
> > +#define ACT8945A_APCH_CTRL 0x79
> > +#define ACT8945A_APCH_STATE 0x7A
> > +
> > +/* ACT8945A_APCH_CFG */
> > +#define APCH_CFG_OVPSET (0x03 << 0)
> > +#define APCH_CFG_OVPSET_6V6 (0x0 << 0)
> > +#define APCH_CFG_OVPSET_7V (0x1 << 0)
> > +#define APCH_CFG_OVPSET_7V5 (0x2 << 0)
> > +#define APCH_CFG_OVPSET_8V (0x3 << 0)
> > +#define APCH_CFG_PRETIMO (0x03 << 2)
> > +#define APCH_CFG_PRETIMO_40_MIN (0x0 << 2)
> > +#define APCH_CFG_PRETIMO_60_MIN (0x1 << 2)
> > +#define APCH_CFG_PRETIMO_80_MIN (0x2 << 2)
> > +#define APCH_CFG_PRETIMO_DISABLED (0x3 << 2)
> > +#define APCH_CFG_TOTTIMO (0x03 << 4)
> > +#define APCH_CFG_TOTTIMO_3_HOUR (0x0 << 4)
> > +#define APCH_CFG_TOTTIMO_4_HOUR (0x1 << 4)
> > +#define APCH_CFG_TOTTIMO_5_HOUR (0x2 << 4)
> > +#define APCH_CFG_TOTTIMO_DISABLED (0x3 << 4)
> > +#define APCH_CFG_SUSCHG (0x01 << 7)
> > +
> > +#define APCH_STATUS_CHGDAT (0x01 << 0)
> > +#define APCH_STATUS_INDAT (0x01 << 1)
> > +#define APCH_STATUS_TEMPDAT (0x01 << 2)
> > +#define APCH_STATUS_TIMRDAT (0x01 << 3)
> > +#define APCH_STATUS_CHGSTAT (0x01 << 4)
> > +#define APCH_STATUS_INSTAT (0x01 << 5)
> > +#define APCH_STATUS_TEMPSTAT (0x01 << 6)
> > +#define APCH_STATUS_TIMRSTAT (0x01 << 7)
> > +
> > +#define APCH_CTRL_CHGEOCOUT (0x01 << 0)
> > +#define APCH_CTRL_INDIS (0x01 << 1)
> > +#define APCH_CTRL_TEMPOUT (0x01 << 2)
> > +#define APCH_CTRL_TIMRPRE (0x01 << 3)
> > +#define APCH_CTRL_CHGEOCIN (0x01 << 4)
> > +#define APCH_CTRL_INCON (0x01 << 5)
> > +#define APCH_CTRL_TEMPIN (0x01 << 6)
> > +#define APCH_CTRL_TIMRTOT (0x01 << 7)
> > +
> > +#define APCH_STATE_ACINSTAT (0x01 << 1)
> > +#define APCH_STATE_CSTATE (0x03 << 4)
> > +#define APCH_STATE_CSTATE_SHIFT 4
> > +#define APCH_STATE_CSTATE_DISABLED 0x00
> > +#define APCH_STATE_CSTATE_EOC 0x01
> > +#define APCH_STATE_CSTATE_FAST 0x02
> > +#define APCH_STATE_CSTATE_PRE 0x03
> > +
> > +struct act8945a_charger {
> > + struct device *dev;
> > + struct act8945a_dev *act8945a_dev;
> > + struct power_supply *psy;
> > +
> > + u32 tolal_time_out;
> > + u32 pre_time_out;
> > + u32 input_voltage_threshold;
> > + bool battery_temperature;
> > + int chglev_pin;
> > + int chglev_value;
> > +};
> > +
> > +static int act8945a_get_charger_state(struct regmap *regmap, int
> > +*val) {
> > + int ret;
> > + unsigned int status, state;
> > +
> > + ret = regmap_read(regmap, ACT8945A_APCH_STATUS, &status);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = regmap_read(regmap, ACT8945A_APCH_STATE, &state);
> > + if (ret < 0)
> > + return ret;
> > +
> > + state &= APCH_STATE_CSTATE;
> > + state >>= APCH_STATE_CSTATE_SHIFT;
> > +
> > + if (state == APCH_STATE_CSTATE_EOC) {
> > + if (status & APCH_STATUS_CHGDAT)
> > + *val = POWER_SUPPLY_STATUS_FULL;
> > + else
> > + *val = POWER_SUPPLY_STATUS_NOT_CHARGING;
> > + } else if ((state == APCH_STATE_CSTATE_FAST) ||
> > + (state == APCH_STATE_CSTATE_PRE)) {
> > + *val = POWER_SUPPLY_STATUS_CHARGING;
> > + } else {
> > + *val = POWER_SUPPLY_STATUS_NOT_CHARGING;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int act8945a_get_charge_type(struct regmap *regmap, int *val)
> > +{
> > + int ret;
> > + unsigned int state;
> > +
> > + ret = regmap_read(regmap, ACT8945A_APCH_STATE, &state);
> > + if (ret < 0)
> > + return ret;
> > +
> > + state &= APCH_STATE_CSTATE;
> > + state >>= APCH_STATE_CSTATE_SHIFT;
> > +
> > + switch (state) {
> > + case APCH_STATE_CSTATE_PRE:
> > + *val = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
> > + break;
> > + case APCH_STATE_CSTATE_FAST:
> > + *val = POWER_SUPPLY_CHARGE_TYPE_FAST;
> > + break;
> > + case APCH_STATE_CSTATE_EOC:
> > + case APCH_STATE_CSTATE_DISABLED:
> > + default:
> > + *val = POWER_SUPPLY_CHARGE_TYPE_NONE;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int act8945a_get_battery_health(struct act8945a_charger *charger,
> > + struct regmap *regmap, int *val)
> > +{
> > + int ret;
> > + unsigned int status;
> > +
> > + ret = regmap_read(regmap, ACT8945A_APCH_STATUS, &status);
> > + if (ret < 0)
> > + return ret;
> > +
> > + if (charger->battery_temperature && !(status &
> APCH_STATUS_TEMPDAT))
> > + *val = POWER_SUPPLY_HEALTH_OVERHEAT;
> > + else if (!(status & APCH_STATUS_INDAT))
> > + *val = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
> > + else if (status & APCH_STATUS_TIMRDAT)
> > + *val = POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE;
> > + else
> > + *val = POWER_SUPPLY_HEALTH_GOOD;
> > +
> > + return 0;
> > +}
> > +
> > +static enum power_supply_property act8945a_charger_props[] = {
> > + POWER_SUPPLY_PROP_STATUS,
> > + POWER_SUPPLY_PROP_CHARGE_TYPE,
> > + POWER_SUPPLY_PROP_TECHNOLOGY,
> > + POWER_SUPPLY_PROP_HEALTH,
> > + POWER_SUPPLY_PROP_MODEL_NAME,
> > + POWER_SUPPLY_PROP_MANUFACTURER };
> > +
> > +static int act8945a_charger_get_property(struct power_supply *psy,
> > + enum power_supply_property prop,
> > + union power_supply_propval *val) {
> > + struct act8945a_charger *charger = power_supply_get_drvdata(psy);
> > + struct regmap *regmap = charger->act8945a_dev->regmap;
> > + int ret = 0;
> > +
> > + switch (prop) {
> > + case POWER_SUPPLY_PROP_STATUS:
> > + ret = act8945a_get_charger_state(regmap, &val->intval);
> > + break;
> > + case POWER_SUPPLY_PROP_CHARGE_TYPE:
> > + ret = act8945a_get_charge_type(regmap, &val->intval);
> > + break;
> > + case POWER_SUPPLY_PROP_TECHNOLOGY:
> > + val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
> > + break;
> > + case POWER_SUPPLY_PROP_HEALTH:
> > + ret = act8945a_get_battery_health(charger,
> > + regmap, &val->intval);
> > + break;
> > + case POWER_SUPPLY_PROP_MODEL_NAME:
> > + val->strval = act8945a_charger_model;
> > + break;
> > + case POWER_SUPPLY_PROP_MANUFACTURER:
> > + val->strval = act8945a_charger_manufacturer;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static const struct power_supply_desc act8945a_charger_desc = {
> > + .name = "act8945a-charger",
> > + .type = POWER_SUPPLY_TYPE_BATTERY,
> > + .get_property = act8945a_charger_get_property,
> > + .properties = act8945a_charger_props,
> > + .num_properties = ARRAY_SIZE(act8945a_charger_props),
> > +};
> > +
> > +#define DEFAULT_TOTAL_TIME_OUT 3
> > +#define DEFAULT_PRE_TIME_OUT 40
> > +#define DEFAULT_INPUT_OVP_THRESHOLD 6600
> > +
> > +static int act8945a_charger_parse_dt(struct device *dev,
> > + struct act8945a_charger *charger)
> > +{
> > + struct device_node *np = dev->of_node;
> > + enum of_gpio_flags flags;
> > +
> > + if (!np) {
> > + dev_err(dev, "no charger of node\n");
> > + return -EINVAL;
> > + }
> > +
> > + charger->chglev_pin = of_get_named_gpio_flags(np,
> > + "active-semi,chglev-gpio", 0, &flags);
> > +
> > + charger->chglev_value = (flags == OF_GPIO_ACTIVE_LOW) ? 0 : 1;
> > +
> > + charger->battery_temperature = !!of_get_property(np,
> > + "active-semi,battery_temperature",
> > + NULL);
>
> Just: of_property_read_bool()?
>
> > +
> > + if (of_property_read_u32(np, "active-semi,input_voltage_threshold",
> > + &charger->input_voltage_threshold))
> > + charger->input_voltage_threshold =
> > + DEFAULT_PRE_TIME_OUT;
> > +
> > + if (of_property_read_u32(np, "active-semi,precondition_timeout",
> > + &charger->pre_time_out))
> > + charger->pre_time_out = DEFAULT_PRE_TIME_OUT;
> > +
> > + if (of_property_read_u32(np, "active-semi,total_timeout",
> > + &charger->tolal_time_out))
> > + charger->tolal_time_out = DEFAULT_TOTAL_TIME_OUT;
> > +
> > + return 0;
> > +}
> > +
> > +static int act8945a_charger_config(struct act8945a_charger *charger)
> > +{
> > + struct regmap *regmap = charger->act8945a_dev->regmap;
> > + u8 value = 0;
> > +
> > + if (gpio_is_valid(charger->chglev_pin))
> > + gpio_set_value(charger->chglev_pin,
> > + charger->chglev_value);
> > +
> > + switch (charger->input_voltage_threshold) {
> > + case 8000:
> > + value |= APCH_CFG_OVPSET_8V;
> > + break;
> > + case 7500:
> > + value |= APCH_CFG_OVPSET_7V5;
> > + break;
> > + case 7000:
> > + value |= APCH_CFG_OVPSET_7V;
> > + break;
> > + case 6600:
> > + default:
> > + value |= APCH_CFG_OVPSET_6V6;
> > + break;
> > + }
> > +
> > + switch (charger->pre_time_out) {
> > + case 60:
> > + value |= APCH_CFG_PRETIMO_60_MIN;
> > + break;
> > + case 80:
> > + value |= APCH_CFG_PRETIMO_80_MIN;
> > + break;
> > + case 0:
> > + value |= APCH_CFG_PRETIMO_DISABLED;
> > + break;
> > + case 40:
> > + default:
> > + value |= APCH_CFG_PRETIMO_40_MIN;
> > + break;
> > + }
> > +
> > + switch (charger->tolal_time_out) {
> > + case 4:
> > + value |= APCH_CFG_TOTTIMO_4_HOUR;
> > + break;
> > + case 5:
> > + value |= APCH_CFG_TOTTIMO_5_HOUR;
> > + break;
> > + case 0:
> > + value |= APCH_CFG_TOTTIMO_DISABLED;
> > + break;
> > + case 3:
> > + default:
> > + value |= APCH_CFG_TOTTIMO_3_HOUR;
> > + break;
> > + }
> > +
> > + return regmap_write(regmap, ACT8945A_APCH_CFG, value); }
> > +
> > +static int act8945a_charger_probe(struct platform_device *pdev) {
> > + struct act8945a_dev *act8945a_dev = dev_get_drvdata(pdev->dev.parent);
> > + struct act8945a_charger *charger;
> > + struct power_supply_config psy_cfg = {};
> > + int ret;
> > +
> > + charger = devm_kzalloc(&pdev->dev, sizeof(*charger), GFP_KERNEL);
> > + if (!charger)
> > + return -ENOMEM;
> > +
> > + platform_set_drvdata(pdev, charger);
> > +
> > + charger->dev = &pdev->dev;
> > + charger->act8945a_dev = act8945a_dev;
> > +
> > + ret = act8945a_charger_parse_dt(&pdev->dev, charger);
> > + if (ret)
> > + return ret;
> > +
> > + ret = act8945a_charger_config(charger);
> > + if (ret)
> > + return ret;
> > +
> > + psy_cfg.of_node = pdev->dev.of_node;
> > + psy_cfg.drv_data = charger;
> > +
> > + charger->psy = power_supply_register(&pdev->dev,
> > + &act8945a_charger_desc,
> > + &psy_cfg);
>
> You can use devm-like() API for this and get rid of remove function...
> but it is not a necessity I think. Up to you.
>
> > + if (IS_ERR(charger->psy)) {
> > + dev_err(act8945a_dev->dev, "failed to register power supply\n");
> > + return PTR_ERR(charger->psy);
> > + }
> > +
> > + dev_info(act8945a_dev->dev, "charger driver registered\n");
> > +
> > + return 0;
> > +}
> > +
> > +static int act8945a_charger_remove(struct platform_device *pdev) {
> > + struct act8945a_charger *charger = platform_get_drvdata(pdev);
> > +
> > + power_supply_unregister(charger->psy);
> > +
> > + return 0;
> > +}
> > +
> > +static struct platform_driver act8945a_charger_driver = {
> > + .driver = {
> > + .name = "act8945a-charger",
> > + },
> > + .probe = act8945a_charger_probe,
> > + .remove = act8945a_charger_remove, };
> > +
> > +static int __init act8945a_charger_init(void) {
> > + return platform_driver_register(&act8945a_charger_driver);
> > +}
> > +subsys_initcall(act8945a_charger_init);
>
> Why subsys_initcall? This should be regular module_platform_driver.
I want it to register early enough as MFD. Maybe I am wrong.
>
> > +
> > +static void __exit act8945a_charger_cleanup(void) {
> > + platform_driver_unregister(&act8945a_charger_driver);
> > +}
> > +module_exit(act8945a_charger_cleanup);
> > +
> > +MODULE_DESCRIPTION("Active-semi ACT8945A ActivePath charger driver");
> > +MODULE_AUTHOR("Wenyou Yang <wenyou.yang@atmel.com>");
> > +MODULE_LICENSE("GPL v2");
>
> In the comment at beginning you mentioned GPL v2+ so this should be "GPL".
>
> Best regards,
> Krzysztof
Best Regards,
Wenyou Yang
^ permalink raw reply [flat|nested] 29+ messages in thread* RE: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
@ 2016-01-08 6:43 ` Yang, Wenyou
0 siblings, 0 replies; 29+ messages in thread
From: Yang, Wenyou @ 2016-01-08 6:43 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Ferre, Nicolas, linux-arm-kernel, linux-kernel, linux-pm
Hello Krzysztof,
Thank you for your commends.
They will be fixed in the next version.
> -----Original Message-----
> From: k.kozlowski.k@gmail.com [mailto:k.kozlowski.k@gmail.com] On Behalf Of
> Krzysztof Kozlowski
> Sent: 2016年1月8日 11:55
> To: Yang, Wenyou <Wenyou.Yang@atmel.com>
> Cc: Sebastian Reichel <sre@kernel.org>; Dmitry Eremin-Solenikov
> <dbaryshkov@gmail.com>; David Woodhouse <dwmw2@infradead.org>; Rob
> Herring <robh+dt@kernel.org>; Pawel Moll <pawel.moll@arm.com>; Mark
> Rutland <mark.rutland@arm.com>; Ian Campbell <ijc+devicetree@hellion.org.uk>;
> Kumar Gala <galak@codeaurora.org>; Ferre, Nicolas
> <Nicolas.FERRE@atmel.com>; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; linux-pm@vger.kernel.org
> Subject: Re: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
>
> 2016-01-08 11:13 GMT+09:00 Wenyou Yang <wenyou.yang@atmel.com>:
> > This patch adds new driver for Active-semi ACT8945A ActivePath charger
> > (part of ACT8945A MFD driver) providing power supply class information
> > to userspace.
> >
> > The driver is configured through DTS (battery and system related
> > settings) and sysfs entries (timers and input over-voltage threshold).
> >
> > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> > ---
> >
> > drivers/power/Kconfig | 7 +
> > drivers/power/Makefile | 1 +
> > drivers/power/act8945a_charger.c | 397
> > ++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 405 insertions(+)
> > create mode 100644 drivers/power/act8945a_charger.c
> >
> > diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig index
> > 1ddd13c..ae75211 100644
> > --- a/drivers/power/Kconfig
> > +++ b/drivers/power/Kconfig
> > @@ -75,6 +75,13 @@ config BATTERY_88PM860X
> > help
> > Say Y here to enable battery monitor for Marvell 88PM860x chip.
> >
> > +config BATTERY_ACT8945A
> > + tristate "Active-semi ACT8945A charger driver"
> > + depends on MFD_ACT8945A
> > + help
> > + Say Y here to enable support for power supply provided by
> > + Active-semi ActivePath ACT8945A charger.
> > +
> > config BATTERY_DS2760
> > tristate "DS2760 battery driver (HP iPAQ & others)"
> > depends on W1 && W1_SLAVE_DS2760 diff --git
> > a/drivers/power/Makefile b/drivers/power/Makefile index
> > 0e4eab5..e46b75d 100644
> > --- a/drivers/power/Makefile
> > +++ b/drivers/power/Makefile
> > @@ -17,6 +17,7 @@ obj-$(CONFIG_WM8350_POWER) +=
> wm8350_power.o
> > obj-$(CONFIG_TEST_POWER) += test_power.o
> >
> > obj-$(CONFIG_BATTERY_88PM860X) += 88pm860x_battery.o
> > +obj-$(CONFIG_BATTERY_ACT8945A) += act8945a_charger.o
> > obj-$(CONFIG_BATTERY_DS2760) += ds2760_battery.o
> > obj-$(CONFIG_BATTERY_DS2780) += ds2780_battery.o
> > obj-$(CONFIG_BATTERY_DS2781) += ds2781_battery.o
> > diff --git a/drivers/power/act8945a_charger.c
> > b/drivers/power/act8945a_charger.c
> > new file mode 100644
> > index 0000000..5b55cd7
> > --- /dev/null
> > +++ b/drivers/power/act8945a_charger.c
> > @@ -0,0 +1,397 @@
> > +/*
> > + * Power supply driver for the Active-semi ACT8945A PMIC
> > + *
> > + * Copyright (C) 2015 Atmel Corporation
> > + * Wenyou Yang <wenyou.yang@atmel.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > +modify
> > + * it under the terms of the GNU General Public License as published
> > +by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
> the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/mfd/act8945a.h>
> > +#include <linux/of.h>
> > +#include <linux/of_gpio.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/power_supply.h>
> > +#include <linux/regmap.h>
> > +
> > +static const char *act8945a_charger_model = "ACT8945A"; static const
> > +char *act8945a_charger_manufacturer = "Active-semi";
> > +
> > +/**
> > + * ACT8945A Charger Register Map
> > + */
> > +
> > +/* 0x70: Reserved */
> > +#define ACT8945A_APCH_CFG 0x71
> > +#define ACT8945A_APCH_STATUS 0x78
> > +#define ACT8945A_APCH_CTRL 0x79
> > +#define ACT8945A_APCH_STATE 0x7A
> > +
> > +/* ACT8945A_APCH_CFG */
> > +#define APCH_CFG_OVPSET (0x03 << 0)
> > +#define APCH_CFG_OVPSET_6V6 (0x0 << 0)
> > +#define APCH_CFG_OVPSET_7V (0x1 << 0)
> > +#define APCH_CFG_OVPSET_7V5 (0x2 << 0)
> > +#define APCH_CFG_OVPSET_8V (0x3 << 0)
> > +#define APCH_CFG_PRETIMO (0x03 << 2)
> > +#define APCH_CFG_PRETIMO_40_MIN (0x0 << 2)
> > +#define APCH_CFG_PRETIMO_60_MIN (0x1 << 2)
> > +#define APCH_CFG_PRETIMO_80_MIN (0x2 << 2)
> > +#define APCH_CFG_PRETIMO_DISABLED (0x3 << 2)
> > +#define APCH_CFG_TOTTIMO (0x03 << 4)
> > +#define APCH_CFG_TOTTIMO_3_HOUR (0x0 << 4)
> > +#define APCH_CFG_TOTTIMO_4_HOUR (0x1 << 4)
> > +#define APCH_CFG_TOTTIMO_5_HOUR (0x2 << 4)
> > +#define APCH_CFG_TOTTIMO_DISABLED (0x3 << 4)
> > +#define APCH_CFG_SUSCHG (0x01 << 7)
> > +
> > +#define APCH_STATUS_CHGDAT (0x01 << 0)
> > +#define APCH_STATUS_INDAT (0x01 << 1)
> > +#define APCH_STATUS_TEMPDAT (0x01 << 2)
> > +#define APCH_STATUS_TIMRDAT (0x01 << 3)
> > +#define APCH_STATUS_CHGSTAT (0x01 << 4)
> > +#define APCH_STATUS_INSTAT (0x01 << 5)
> > +#define APCH_STATUS_TEMPSTAT (0x01 << 6)
> > +#define APCH_STATUS_TIMRSTAT (0x01 << 7)
> > +
> > +#define APCH_CTRL_CHGEOCOUT (0x01 << 0)
> > +#define APCH_CTRL_INDIS (0x01 << 1)
> > +#define APCH_CTRL_TEMPOUT (0x01 << 2)
> > +#define APCH_CTRL_TIMRPRE (0x01 << 3)
> > +#define APCH_CTRL_CHGEOCIN (0x01 << 4)
> > +#define APCH_CTRL_INCON (0x01 << 5)
> > +#define APCH_CTRL_TEMPIN (0x01 << 6)
> > +#define APCH_CTRL_TIMRTOT (0x01 << 7)
> > +
> > +#define APCH_STATE_ACINSTAT (0x01 << 1)
> > +#define APCH_STATE_CSTATE (0x03 << 4)
> > +#define APCH_STATE_CSTATE_SHIFT 4
> > +#define APCH_STATE_CSTATE_DISABLED 0x00
> > +#define APCH_STATE_CSTATE_EOC 0x01
> > +#define APCH_STATE_CSTATE_FAST 0x02
> > +#define APCH_STATE_CSTATE_PRE 0x03
> > +
> > +struct act8945a_charger {
> > + struct device *dev;
> > + struct act8945a_dev *act8945a_dev;
> > + struct power_supply *psy;
> > +
> > + u32 tolal_time_out;
> > + u32 pre_time_out;
> > + u32 input_voltage_threshold;
> > + bool battery_temperature;
> > + int chglev_pin;
> > + int chglev_value;
> > +};
> > +
> > +static int act8945a_get_charger_state(struct regmap *regmap, int
> > +*val) {
> > + int ret;
> > + unsigned int status, state;
> > +
> > + ret = regmap_read(regmap, ACT8945A_APCH_STATUS, &status);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = regmap_read(regmap, ACT8945A_APCH_STATE, &state);
> > + if (ret < 0)
> > + return ret;
> > +
> > + state &= APCH_STATE_CSTATE;
> > + state >>= APCH_STATE_CSTATE_SHIFT;
> > +
> > + if (state == APCH_STATE_CSTATE_EOC) {
> > + if (status & APCH_STATUS_CHGDAT)
> > + *val = POWER_SUPPLY_STATUS_FULL;
> > + else
> > + *val = POWER_SUPPLY_STATUS_NOT_CHARGING;
> > + } else if ((state == APCH_STATE_CSTATE_FAST) ||
> > + (state == APCH_STATE_CSTATE_PRE)) {
> > + *val = POWER_SUPPLY_STATUS_CHARGING;
> > + } else {
> > + *val = POWER_SUPPLY_STATUS_NOT_CHARGING;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int act8945a_get_charge_type(struct regmap *regmap, int *val)
> > +{
> > + int ret;
> > + unsigned int state;
> > +
> > + ret = regmap_read(regmap, ACT8945A_APCH_STATE, &state);
> > + if (ret < 0)
> > + return ret;
> > +
> > + state &= APCH_STATE_CSTATE;
> > + state >>= APCH_STATE_CSTATE_SHIFT;
> > +
> > + switch (state) {
> > + case APCH_STATE_CSTATE_PRE:
> > + *val = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
> > + break;
> > + case APCH_STATE_CSTATE_FAST:
> > + *val = POWER_SUPPLY_CHARGE_TYPE_FAST;
> > + break;
> > + case APCH_STATE_CSTATE_EOC:
> > + case APCH_STATE_CSTATE_DISABLED:
> > + default:
> > + *val = POWER_SUPPLY_CHARGE_TYPE_NONE;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int act8945a_get_battery_health(struct act8945a_charger *charger,
> > + struct regmap *regmap, int *val)
> > +{
> > + int ret;
> > + unsigned int status;
> > +
> > + ret = regmap_read(regmap, ACT8945A_APCH_STATUS, &status);
> > + if (ret < 0)
> > + return ret;
> > +
> > + if (charger->battery_temperature && !(status &
> APCH_STATUS_TEMPDAT))
> > + *val = POWER_SUPPLY_HEALTH_OVERHEAT;
> > + else if (!(status & APCH_STATUS_INDAT))
> > + *val = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
> > + else if (status & APCH_STATUS_TIMRDAT)
> > + *val = POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE;
> > + else
> > + *val = POWER_SUPPLY_HEALTH_GOOD;
> > +
> > + return 0;
> > +}
> > +
> > +static enum power_supply_property act8945a_charger_props[] = {
> > + POWER_SUPPLY_PROP_STATUS,
> > + POWER_SUPPLY_PROP_CHARGE_TYPE,
> > + POWER_SUPPLY_PROP_TECHNOLOGY,
> > + POWER_SUPPLY_PROP_HEALTH,
> > + POWER_SUPPLY_PROP_MODEL_NAME,
> > + POWER_SUPPLY_PROP_MANUFACTURER };
> > +
> > +static int act8945a_charger_get_property(struct power_supply *psy,
> > + enum power_supply_property prop,
> > + union power_supply_propval *val) {
> > + struct act8945a_charger *charger = power_supply_get_drvdata(psy);
> > + struct regmap *regmap = charger->act8945a_dev->regmap;
> > + int ret = 0;
> > +
> > + switch (prop) {
> > + case POWER_SUPPLY_PROP_STATUS:
> > + ret = act8945a_get_charger_state(regmap, &val->intval);
> > + break;
> > + case POWER_SUPPLY_PROP_CHARGE_TYPE:
> > + ret = act8945a_get_charge_type(regmap, &val->intval);
> > + break;
> > + case POWER_SUPPLY_PROP_TECHNOLOGY:
> > + val->intval = POWER_SUPPLY_TECHNOLOGY_LION;
> > + break;
> > + case POWER_SUPPLY_PROP_HEALTH:
> > + ret = act8945a_get_battery_health(charger,
> > + regmap, &val->intval);
> > + break;
> > + case POWER_SUPPLY_PROP_MODEL_NAME:
> > + val->strval = act8945a_charger_model;
> > + break;
> > + case POWER_SUPPLY_PROP_MANUFACTURER:
> > + val->strval = act8945a_charger_manufacturer;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static const struct power_supply_desc act8945a_charger_desc = {
> > + .name = "act8945a-charger",
> > + .type = POWER_SUPPLY_TYPE_BATTERY,
> > + .get_property = act8945a_charger_get_property,
> > + .properties = act8945a_charger_props,
> > + .num_properties = ARRAY_SIZE(act8945a_charger_props),
> > +};
> > +
> > +#define DEFAULT_TOTAL_TIME_OUT 3
> > +#define DEFAULT_PRE_TIME_OUT 40
> > +#define DEFAULT_INPUT_OVP_THRESHOLD 6600
> > +
> > +static int act8945a_charger_parse_dt(struct device *dev,
> > + struct act8945a_charger *charger)
> > +{
> > + struct device_node *np = dev->of_node;
> > + enum of_gpio_flags flags;
> > +
> > + if (!np) {
> > + dev_err(dev, "no charger of node\n");
> > + return -EINVAL;
> > + }
> > +
> > + charger->chglev_pin = of_get_named_gpio_flags(np,
> > + "active-semi,chglev-gpio", 0, &flags);
> > +
> > + charger->chglev_value = (flags == OF_GPIO_ACTIVE_LOW) ? 0 : 1;
> > +
> > + charger->battery_temperature = !!of_get_property(np,
> > + "active-semi,battery_temperature",
> > + NULL);
>
> Just: of_property_read_bool()?
>
> > +
> > + if (of_property_read_u32(np, "active-semi,input_voltage_threshold",
> > + &charger->input_voltage_threshold))
> > + charger->input_voltage_threshold =
> > + DEFAULT_PRE_TIME_OUT;
> > +
> > + if (of_property_read_u32(np, "active-semi,precondition_timeout",
> > + &charger->pre_time_out))
> > + charger->pre_time_out = DEFAULT_PRE_TIME_OUT;
> > +
> > + if (of_property_read_u32(np, "active-semi,total_timeout",
> > + &charger->tolal_time_out))
> > + charger->tolal_time_out = DEFAULT_TOTAL_TIME_OUT;
> > +
> > + return 0;
> > +}
> > +
> > +static int act8945a_charger_config(struct act8945a_charger *charger)
> > +{
> > + struct regmap *regmap = charger->act8945a_dev->regmap;
> > + u8 value = 0;
> > +
> > + if (gpio_is_valid(charger->chglev_pin))
> > + gpio_set_value(charger->chglev_pin,
> > + charger->chglev_value);
> > +
> > + switch (charger->input_voltage_threshold) {
> > + case 8000:
> > + value |= APCH_CFG_OVPSET_8V;
> > + break;
> > + case 7500:
> > + value |= APCH_CFG_OVPSET_7V5;
> > + break;
> > + case 7000:
> > + value |= APCH_CFG_OVPSET_7V;
> > + break;
> > + case 6600:
> > + default:
> > + value |= APCH_CFG_OVPSET_6V6;
> > + break;
> > + }
> > +
> > + switch (charger->pre_time_out) {
> > + case 60:
> > + value |= APCH_CFG_PRETIMO_60_MIN;
> > + break;
> > + case 80:
> > + value |= APCH_CFG_PRETIMO_80_MIN;
> > + break;
> > + case 0:
> > + value |= APCH_CFG_PRETIMO_DISABLED;
> > + break;
> > + case 40:
> > + default:
> > + value |= APCH_CFG_PRETIMO_40_MIN;
> > + break;
> > + }
> > +
> > + switch (charger->tolal_time_out) {
> > + case 4:
> > + value |= APCH_CFG_TOTTIMO_4_HOUR;
> > + break;
> > + case 5:
> > + value |= APCH_CFG_TOTTIMO_5_HOUR;
> > + break;
> > + case 0:
> > + value |= APCH_CFG_TOTTIMO_DISABLED;
> > + break;
> > + case 3:
> > + default:
> > + value |= APCH_CFG_TOTTIMO_3_HOUR;
> > + break;
> > + }
> > +
> > + return regmap_write(regmap, ACT8945A_APCH_CFG, value); }
> > +
> > +static int act8945a_charger_probe(struct platform_device *pdev) {
> > + struct act8945a_dev *act8945a_dev = dev_get_drvdata(pdev->dev.parent);
> > + struct act8945a_charger *charger;
> > + struct power_supply_config psy_cfg = {};
> > + int ret;
> > +
> > + charger = devm_kzalloc(&pdev->dev, sizeof(*charger), GFP_KERNEL);
> > + if (!charger)
> > + return -ENOMEM;
> > +
> > + platform_set_drvdata(pdev, charger);
> > +
> > + charger->dev = &pdev->dev;
> > + charger->act8945a_dev = act8945a_dev;
> > +
> > + ret = act8945a_charger_parse_dt(&pdev->dev, charger);
> > + if (ret)
> > + return ret;
> > +
> > + ret = act8945a_charger_config(charger);
> > + if (ret)
> > + return ret;
> > +
> > + psy_cfg.of_node = pdev->dev.of_node;
> > + psy_cfg.drv_data = charger;
> > +
> > + charger->psy = power_supply_register(&pdev->dev,
> > + &act8945a_charger_desc,
> > + &psy_cfg);
>
> You can use devm-like() API for this and get rid of remove function...
> but it is not a necessity I think. Up to you.
>
> > + if (IS_ERR(charger->psy)) {
> > + dev_err(act8945a_dev->dev, "failed to register power supply\n");
> > + return PTR_ERR(charger->psy);
> > + }
> > +
> > + dev_info(act8945a_dev->dev, "charger driver registered\n");
> > +
> > + return 0;
> > +}
> > +
> > +static int act8945a_charger_remove(struct platform_device *pdev) {
> > + struct act8945a_charger *charger = platform_get_drvdata(pdev);
> > +
> > + power_supply_unregister(charger->psy);
> > +
> > + return 0;
> > +}
> > +
> > +static struct platform_driver act8945a_charger_driver = {
> > + .driver = {
> > + .name = "act8945a-charger",
> > + },
> > + .probe = act8945a_charger_probe,
> > + .remove = act8945a_charger_remove, };
> > +
> > +static int __init act8945a_charger_init(void) {
> > + return platform_driver_register(&act8945a_charger_driver);
> > +}
> > +subsys_initcall(act8945a_charger_init);
>
> Why subsys_initcall? This should be regular module_platform_driver.
I want it to register early enough as MFD. Maybe I am wrong.
>
> > +
> > +static void __exit act8945a_charger_cleanup(void) {
> > + platform_driver_unregister(&act8945a_charger_driver);
> > +}
> > +module_exit(act8945a_charger_cleanup);
> > +
> > +MODULE_DESCRIPTION("Active-semi ACT8945A ActivePath charger driver");
> > +MODULE_AUTHOR("Wenyou Yang <wenyou.yang@atmel.com>");
> > +MODULE_LICENSE("GPL v2");
>
> In the comment at beginning you mentioned GPL v2+ so this should be "GPL".
>
> Best regards,
> Krzysztof
Best Regards,
Wenyou Yang
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
2016-01-08 6:43 ` Yang, Wenyou
(?)
@ 2016-01-08 7:02 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 29+ messages in thread
From: Krzysztof Kozlowski @ 2016-01-08 7:02 UTC (permalink / raw)
To: Yang, Wenyou
Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Ferre, Nicolas, linux-arm-kernel, linux-kernel, linux-pm
On 08.01.2016 15:43, Yang, Wenyou wrote:
[...]
>>> +static struct platform_driver act8945a_charger_driver = {
>>> + .driver = {
>>> + .name = "act8945a-charger",
>>> + },
>>> + .probe = act8945a_charger_probe,
>>> + .remove = act8945a_charger_remove, };
>>> +
>>> +static int __init act8945a_charger_init(void) {
>>> + return platform_driver_register(&act8945a_charger_driver);
>>> +}
>>> +subsys_initcall(act8945a_charger_init);
>>
>> Why subsys_initcall? This should be regular module_platform_driver.
>
> I want it to register early enough as MFD. Maybe I am wrong.
No, you shouldn't manually order the probing by initcalls. This should
be registered as usual and, if needed, support deferred probing. In this
case I even can't find any reason to register it earlier than usual.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
@ 2016-01-08 7:02 ` Krzysztof Kozlowski
0 siblings, 0 replies; 29+ messages in thread
From: Krzysztof Kozlowski @ 2016-01-08 7:02 UTC (permalink / raw)
To: linux-arm-kernel
On 08.01.2016 15:43, Yang, Wenyou wrote:
[...]
>>> +static struct platform_driver act8945a_charger_driver = {
>>> + .driver = {
>>> + .name = "act8945a-charger",
>>> + },
>>> + .probe = act8945a_charger_probe,
>>> + .remove = act8945a_charger_remove, };
>>> +
>>> +static int __init act8945a_charger_init(void) {
>>> + return platform_driver_register(&act8945a_charger_driver);
>>> +}
>>> +subsys_initcall(act8945a_charger_init);
>>
>> Why subsys_initcall? This should be regular module_platform_driver.
>
> I want it to register early enough as MFD. Maybe I am wrong.
No, you shouldn't manually order the probing by initcalls. This should
be registered as usual and, if needed, support deferred probing. In this
case I even can't find any reason to register it earlier than usual.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
@ 2016-01-08 7:02 ` Krzysztof Kozlowski
0 siblings, 0 replies; 29+ messages in thread
From: Krzysztof Kozlowski @ 2016-01-08 7:02 UTC (permalink / raw)
To: Yang, Wenyou
Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Ferre, Nicolas, linux-arm-kernel, linux-kernel, linux-pm
On 08.01.2016 15:43, Yang, Wenyou wrote:
[...]
>>> +static struct platform_driver act8945a_charger_driver = {
>>> + .driver = {
>>> + .name = "act8945a-charger",
>>> + },
>>> + .probe = act8945a_charger_probe,
>>> + .remove = act8945a_charger_remove, };
>>> +
>>> +static int __init act8945a_charger_init(void) {
>>> + return platform_driver_register(&act8945a_charger_driver);
>>> +}
>>> +subsys_initcall(act8945a_charger_init);
>>
>> Why subsys_initcall? This should be regular module_platform_driver.
>
> I want it to register early enough as MFD. Maybe I am wrong.
No, you shouldn't manually order the probing by initcalls. This should
be registered as usual and, if needed, support deferred probing. In this
case I even can't find any reason to register it earlier than usual.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 29+ messages in thread* RE: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
2016-01-08 7:02 ` Krzysztof Kozlowski
(?)
@ 2016-01-08 7:58 ` Yang, Wenyou
-1 siblings, 0 replies; 29+ messages in thread
From: Yang, Wenyou @ 2016-01-08 7:58 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Ferre, Nicolas, linux-arm-kernel, linux-kernel, linux-pm
Hello Krzysztof,
> -----Original Message-----
> From: Krzysztof Kozlowski [mailto:k.kozlowski@samsung.com]
> Sent: 2016年1月8日 15:03
> To: Yang, Wenyou <Wenyou.Yang@atmel.com>
> Cc: Sebastian Reichel <sre@kernel.org>; Dmitry Eremin-Solenikov
> <dbaryshkov@gmail.com>; David Woodhouse <dwmw2@infradead.org>; Rob
> Herring <robh+dt@kernel.org>; Pawel Moll <pawel.moll@arm.com>; Mark
> Rutland <mark.rutland@arm.com>; Ian Campbell <ijc+devicetree@hellion.org.uk>;
> Kumar Gala <galak@codeaurora.org>; Ferre, Nicolas
> <Nicolas.FERRE@atmel.com>; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; linux-pm@vger.kernel.org
> Subject: Re: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
>
> On 08.01.2016 15:43, Yang, Wenyou wrote:
>
> [...]
>
> >>> +static struct platform_driver act8945a_charger_driver = {
> >>> + .driver = {
> >>> + .name = "act8945a-charger",
> >>> + },
> >>> + .probe = act8945a_charger_probe,
> >>> + .remove = act8945a_charger_remove, };
> >>> +
> >>> +static int __init act8945a_charger_init(void) {
> >>> + return platform_driver_register(&act8945a_charger_driver);
> >>> +}
> >>> +subsys_initcall(act8945a_charger_init);
> >>
> >> Why subsys_initcall? This should be regular module_platform_driver.
> >
> > I want it to register early enough as MFD. Maybe I am wrong.
>
> No, you shouldn't manually order the probing by initcalls. This should be
> registered as usual and, if needed, support deferred probing. In this case I even
> can't find any reason to register it earlier than usual.
OK, I will change it.
Thanks.
Best Regards,
Wenyou Yang
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
@ 2016-01-08 7:58 ` Yang, Wenyou
0 siblings, 0 replies; 29+ messages in thread
From: Yang, Wenyou @ 2016-01-08 7:58 UTC (permalink / raw)
To: linux-arm-kernel
Hello Krzysztof,
> -----Original Message-----
> From: Krzysztof Kozlowski [mailto:k.kozlowski at samsung.com]
> Sent: 2016?1?8? 15:03
> To: Yang, Wenyou <Wenyou.Yang@atmel.com>
> Cc: Sebastian Reichel <sre@kernel.org>; Dmitry Eremin-Solenikov
> <dbaryshkov@gmail.com>; David Woodhouse <dwmw2@infradead.org>; Rob
> Herring <robh+dt@kernel.org>; Pawel Moll <pawel.moll@arm.com>; Mark
> Rutland <mark.rutland@arm.com>; Ian Campbell <ijc+devicetree@hellion.org.uk>;
> Kumar Gala <galak@codeaurora.org>; Ferre, Nicolas
> <Nicolas.FERRE@atmel.com>; linux-arm-kernel at lists.infradead.org; linux-
> kernel at vger.kernel.org; linux-pm at vger.kernel.org
> Subject: Re: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
>
> On 08.01.2016 15:43, Yang, Wenyou wrote:
>
> [...]
>
> >>> +static struct platform_driver act8945a_charger_driver = {
> >>> + .driver = {
> >>> + .name = "act8945a-charger",
> >>> + },
> >>> + .probe = act8945a_charger_probe,
> >>> + .remove = act8945a_charger_remove, };
> >>> +
> >>> +static int __init act8945a_charger_init(void) {
> >>> + return platform_driver_register(&act8945a_charger_driver);
> >>> +}
> >>> +subsys_initcall(act8945a_charger_init);
> >>
> >> Why subsys_initcall? This should be regular module_platform_driver.
> >
> > I want it to register early enough as MFD. Maybe I am wrong.
>
> No, you shouldn't manually order the probing by initcalls. This should be
> registered as usual and, if needed, support deferred probing. In this case I even
> can't find any reason to register it earlier than usual.
OK, I will change it.
Thanks.
Best Regards,
Wenyou Yang
^ permalink raw reply [flat|nested] 29+ messages in thread* RE: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
@ 2016-01-08 7:58 ` Yang, Wenyou
0 siblings, 0 replies; 29+ messages in thread
From: Yang, Wenyou @ 2016-01-08 7:58 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Ferre, Nicolas, linux-arm-kernel, linux-kernel, linux-pm
Hello Krzysztof,
> -----Original Message-----
> From: Krzysztof Kozlowski [mailto:k.kozlowski@samsung.com]
> Sent: 2016年1月8日 15:03
> To: Yang, Wenyou <Wenyou.Yang@atmel.com>
> Cc: Sebastian Reichel <sre@kernel.org>; Dmitry Eremin-Solenikov
> <dbaryshkov@gmail.com>; David Woodhouse <dwmw2@infradead.org>; Rob
> Herring <robh+dt@kernel.org>; Pawel Moll <pawel.moll@arm.com>; Mark
> Rutland <mark.rutland@arm.com>; Ian Campbell <ijc+devicetree@hellion.org.uk>;
> Kumar Gala <galak@codeaurora.org>; Ferre, Nicolas
> <Nicolas.FERRE@atmel.com>; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; linux-pm@vger.kernel.org
> Subject: Re: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
>
> On 08.01.2016 15:43, Yang, Wenyou wrote:
>
> [...]
>
> >>> +static struct platform_driver act8945a_charger_driver = {
> >>> + .driver = {
> >>> + .name = "act8945a-charger",
> >>> + },
> >>> + .probe = act8945a_charger_probe,
> >>> + .remove = act8945a_charger_remove, };
> >>> +
> >>> +static int __init act8945a_charger_init(void) {
> >>> + return platform_driver_register(&act8945a_charger_driver);
> >>> +}
> >>> +subsys_initcall(act8945a_charger_init);
> >>
> >> Why subsys_initcall? This should be regular module_platform_driver.
> >
> > I want it to register early enough as MFD. Maybe I am wrong.
>
> No, you shouldn't manually order the probing by initcalls. This should be
> registered as usual and, if needed, support deferred probing. In this case I even
> can't find any reason to register it earlier than usual.
OK, I will change it.
Thanks.
Best Regards,
Wenyou Yang
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
2016-01-08 2:13 ` Wenyou Yang
(?)
@ 2016-01-08 15:00 ` Javier Martinez Canillas
-1 siblings, 0 replies; 29+ messages in thread
From: Javier Martinez Canillas @ 2016-01-08 15:00 UTC (permalink / raw)
To: Wenyou Yang
Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Nicolas Ferre, linux-arm-kernel, Linux Kernel, linux-pm
Hello Wenyou,
On Thu, Jan 7, 2016 at 11:13 PM, Wenyou Yang <wenyou.yang@atmel.com> wrote:
> This patch adds new driver for Active-semi ACT8945A ActivePath
> charger (part of ACT8945A MFD driver) providing power supply class
> information to userspace.
>
> The driver is configured through DTS (battery and system related
> settings) and sysfs entries (timers and input over-voltage threshold).
>
> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> ---
>
[snip]
> +
> +static struct platform_driver act8945a_charger_driver = {
> + .driver = {
> + .name = "act8945a-charger",
> + },
> + .probe = act8945a_charger_probe,
> + .remove = act8945a_charger_remove,
> +};
> +
The driver is missing an OF match table (and also a platform ID match
table if non-DT is supported) so this means that module autoloading
will be broken for this driver since the platform uevent function will
report a MODALIAS=of:N*T*Cactive-semi,act8945a-charger
Matching is only working because the platform bus match function use
as a fallback the driver name.
Best regards,
Javier
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
@ 2016-01-08 15:00 ` Javier Martinez Canillas
0 siblings, 0 replies; 29+ messages in thread
From: Javier Martinez Canillas @ 2016-01-08 15:00 UTC (permalink / raw)
To: linux-arm-kernel
Hello Wenyou,
On Thu, Jan 7, 2016 at 11:13 PM, Wenyou Yang <wenyou.yang@atmel.com> wrote:
> This patch adds new driver for Active-semi ACT8945A ActivePath
> charger (part of ACT8945A MFD driver) providing power supply class
> information to userspace.
>
> The driver is configured through DTS (battery and system related
> settings) and sysfs entries (timers and input over-voltage threshold).
>
> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> ---
>
[snip]
> +
> +static struct platform_driver act8945a_charger_driver = {
> + .driver = {
> + .name = "act8945a-charger",
> + },
> + .probe = act8945a_charger_probe,
> + .remove = act8945a_charger_remove,
> +};
> +
The driver is missing an OF match table (and also a platform ID match
table if non-DT is supported) so this means that module autoloading
will be broken for this driver since the platform uevent function will
report a MODALIAS=of:N*T*Cactive-semi,act8945a-charger
Matching is only working because the platform bus match function use
as a fallback the driver name.
Best regards,
Javier
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
@ 2016-01-08 15:00 ` Javier Martinez Canillas
0 siblings, 0 replies; 29+ messages in thread
From: Javier Martinez Canillas @ 2016-01-08 15:00 UTC (permalink / raw)
To: Wenyou Yang
Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Nicolas Ferre, linux-arm-kernel, Linux Kernel, linux-pm
Hello Wenyou,
On Thu, Jan 7, 2016 at 11:13 PM, Wenyou Yang <wenyou.yang@atmel.com> wrote:
> This patch adds new driver for Active-semi ACT8945A ActivePath
> charger (part of ACT8945A MFD driver) providing power supply class
> information to userspace.
>
> The driver is configured through DTS (battery and system related
> settings) and sysfs entries (timers and input over-voltage threshold).
>
> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> ---
>
[snip]
> +
> +static struct platform_driver act8945a_charger_driver = {
> + .driver = {
> + .name = "act8945a-charger",
> + },
> + .probe = act8945a_charger_probe,
> + .remove = act8945a_charger_remove,
> +};
> +
The driver is missing an OF match table (and also a platform ID match
table if non-DT is supported) so this means that module autoloading
will be broken for this driver since the platform uevent function will
report a MODALIAS=of:N*T*Cactive-semi,act8945a-charger
Matching is only working because the platform bus match function use
as a fallback the driver name.
Best regards,
Javier
^ permalink raw reply [flat|nested] 29+ messages in thread* RE: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
2016-01-08 15:00 ` Javier Martinez Canillas
(?)
@ 2016-01-12 8:23 ` Yang, Wenyou
-1 siblings, 0 replies; 29+ messages in thread
From: Yang, Wenyou @ 2016-01-12 8:23 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Ferre, Nicolas, linux-arm-kernel, Linux Kernel, linux-pm
Hi Javier,
Thank you for your review.
> -----Original Message-----
> From: Javier Martinez Canillas [mailto:javier@dowhile0.org]
> Sent: 2016年1月8日 23:00
> To: Yang, Wenyou <Wenyou.Yang@atmel.com>
> Cc: Sebastian Reichel <sre@kernel.org>; Dmitry Eremin-Solenikov
> <dbaryshkov@gmail.com>; David Woodhouse <dwmw2@infradead.org>; Rob
> Herring <robh+dt@kernel.org>; Pawel Moll <pawel.moll@arm.com>; Mark
> Rutland <mark.rutland@arm.com>; Ian Campbell <ijc+devicetree@hellion.org.uk>;
> Kumar Gala <galak@codeaurora.org>; Ferre, Nicolas
> <Nicolas.FERRE@atmel.com>; linux-arm-kernel@lists.infradead.org; Linux
> Kernel <linux-kernel@vger.kernel.org>; linux-pm@vger.kernel.org
> Subject: Re: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
>
> Hello Wenyou,
>
> On Thu, Jan 7, 2016 at 11:13 PM, Wenyou Yang <wenyou.yang@atmel.com>
> wrote:
> > This patch adds new driver for Active-semi ACT8945A ActivePath charger
> > (part of ACT8945A MFD driver) providing power supply class information
> > to userspace.
> >
> > The driver is configured through DTS (battery and system related
> > settings) and sysfs entries (timers and input over-voltage threshold).
> >
> > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> > ---
> >
>
> [snip]
>
> > +
> > +static struct platform_driver act8945a_charger_driver = {
> > + .driver = {
> > + .name = "act8945a-charger",
> > + },
> > + .probe = act8945a_charger_probe,
> > + .remove = act8945a_charger_remove, };
> > +
>
> The driver is missing an OF match table (and also a platform ID match table if
> non-DT is supported) so this means that module autoloading will be broken for this
> driver since the platform uevent function will report a MODALIAS=of:N*T*Cactive-
> semi,act8945a-charger
This is an MFD device, it compatible is declared in the ../mfd/act8945a.c file.
.of_compatible = "active-semi,act8945a-charger".
The non-DT is not supported in this patch. So a platform ID match table is not included.
>
> Matching is only working because the platform bus match function use as a
> fallback the driver name.
>
> Best regards,
> Javier
Best Regards,
Wenyou Yang
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
@ 2016-01-12 8:23 ` Yang, Wenyou
0 siblings, 0 replies; 29+ messages in thread
From: Yang, Wenyou @ 2016-01-12 8:23 UTC (permalink / raw)
To: linux-arm-kernel
Hi Javier,
Thank you for your review.
> -----Original Message-----
> From: Javier Martinez Canillas [mailto:javier at dowhile0.org]
> Sent: 2016?1?8? 23:00
> To: Yang, Wenyou <Wenyou.Yang@atmel.com>
> Cc: Sebastian Reichel <sre@kernel.org>; Dmitry Eremin-Solenikov
> <dbaryshkov@gmail.com>; David Woodhouse <dwmw2@infradead.org>; Rob
> Herring <robh+dt@kernel.org>; Pawel Moll <pawel.moll@arm.com>; Mark
> Rutland <mark.rutland@arm.com>; Ian Campbell <ijc+devicetree@hellion.org.uk>;
> Kumar Gala <galak@codeaurora.org>; Ferre, Nicolas
> <Nicolas.FERRE@atmel.com>; linux-arm-kernel at lists.infradead.org; Linux
> Kernel <linux-kernel@vger.kernel.org>; linux-pm at vger.kernel.org
> Subject: Re: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
>
> Hello Wenyou,
>
> On Thu, Jan 7, 2016 at 11:13 PM, Wenyou Yang <wenyou.yang@atmel.com>
> wrote:
> > This patch adds new driver for Active-semi ACT8945A ActivePath charger
> > (part of ACT8945A MFD driver) providing power supply class information
> > to userspace.
> >
> > The driver is configured through DTS (battery and system related
> > settings) and sysfs entries (timers and input over-voltage threshold).
> >
> > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> > ---
> >
>
> [snip]
>
> > +
> > +static struct platform_driver act8945a_charger_driver = {
> > + .driver = {
> > + .name = "act8945a-charger",
> > + },
> > + .probe = act8945a_charger_probe,
> > + .remove = act8945a_charger_remove, };
> > +
>
> The driver is missing an OF match table (and also a platform ID match table if
> non-DT is supported) so this means that module autoloading will be broken for this
> driver since the platform uevent function will report a MODALIAS=of:N*T*Cactive-
> semi,act8945a-charger
This is an MFD device, it compatible is declared in the ../mfd/act8945a.c file.
.of_compatible = "active-semi,act8945a-charger".
The non-DT is not supported in this patch. So a platform ID match table is not included.
>
> Matching is only working because the platform bus match function use as a
> fallback the driver name.
>
> Best regards,
> Javier
Best Regards,
Wenyou Yang
^ permalink raw reply [flat|nested] 29+ messages in thread* RE: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
@ 2016-01-12 8:23 ` Yang, Wenyou
0 siblings, 0 replies; 29+ messages in thread
From: Yang, Wenyou @ 2016-01-12 8:23 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Ferre, Nicolas, linux-arm-kernel, Linux Kernel, linux-pm
Hi Javier,
Thank you for your review.
> -----Original Message-----
> From: Javier Martinez Canillas [mailto:javier@dowhile0.org]
> Sent: 2016年1月8日 23:00
> To: Yang, Wenyou <Wenyou.Yang@atmel.com>
> Cc: Sebastian Reichel <sre@kernel.org>; Dmitry Eremin-Solenikov
> <dbaryshkov@gmail.com>; David Woodhouse <dwmw2@infradead.org>; Rob
> Herring <robh+dt@kernel.org>; Pawel Moll <pawel.moll@arm.com>; Mark
> Rutland <mark.rutland@arm.com>; Ian Campbell <ijc+devicetree@hellion.org.uk>;
> Kumar Gala <galak@codeaurora.org>; Ferre, Nicolas
> <Nicolas.FERRE@atmel.com>; linux-arm-kernel@lists.infradead.org; Linux
> Kernel <linux-kernel@vger.kernel.org>; linux-pm@vger.kernel.org
> Subject: Re: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
>
> Hello Wenyou,
>
> On Thu, Jan 7, 2016 at 11:13 PM, Wenyou Yang <wenyou.yang@atmel.com>
> wrote:
> > This patch adds new driver for Active-semi ACT8945A ActivePath charger
> > (part of ACT8945A MFD driver) providing power supply class information
> > to userspace.
> >
> > The driver is configured through DTS (battery and system related
> > settings) and sysfs entries (timers and input over-voltage threshold).
> >
> > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> > ---
> >
>
> [snip]
>
> > +
> > +static struct platform_driver act8945a_charger_driver = {
> > + .driver = {
> > + .name = "act8945a-charger",
> > + },
> > + .probe = act8945a_charger_probe,
> > + .remove = act8945a_charger_remove, };
> > +
>
> The driver is missing an OF match table (and also a platform ID match table if
> non-DT is supported) so this means that module autoloading will be broken for this
> driver since the platform uevent function will report a MODALIAS=of:N*T*Cactive-
> semi,act8945a-charger
This is an MFD device, it compatible is declared in the ../mfd/act8945a.c file.
.of_compatible = "active-semi,act8945a-charger".
The non-DT is not supported in this patch. So a platform ID match table is not included.
>
> Matching is only working because the platform bus match function use as a
> fallback the driver name.
>
> Best regards,
> Javier
Best Regards,
Wenyou Yang
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
2016-01-12 8:23 ` Yang, Wenyou
(?)
@ 2016-01-12 14:03 ` Javier Martinez Canillas
-1 siblings, 0 replies; 29+ messages in thread
From: Javier Martinez Canillas @ 2016-01-12 14:03 UTC (permalink / raw)
To: Yang, Wenyou
Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Ferre, Nicolas, linux-arm-kernel, Linux Kernel, linux-pm
Hello Wenyou,
On Tue, Jan 12, 2016 at 5:23 AM, Yang, Wenyou <Wenyou.Yang@atmel.com> wrote:
> Hi Javier,
>
> Thank you for your review.
>
>> -----Original Message-----
>> From: Javier Martinez Canillas [mailto:javier@dowhile0.org]
>> Sent: 2016年1月8日 23:00
>> To: Yang, Wenyou <Wenyou.Yang@atmel.com>
>> Cc: Sebastian Reichel <sre@kernel.org>; Dmitry Eremin-Solenikov
>> <dbaryshkov@gmail.com>; David Woodhouse <dwmw2@infradead.org>; Rob
>> Herring <robh+dt@kernel.org>; Pawel Moll <pawel.moll@arm.com>; Mark
>> Rutland <mark.rutland@arm.com>; Ian Campbell <ijc+devicetree@hellion.org.uk>;
>> Kumar Gala <galak@codeaurora.org>; Ferre, Nicolas
>> <Nicolas.FERRE@atmel.com>; linux-arm-kernel@lists.infradead.org; Linux
>> Kernel <linux-kernel@vger.kernel.org>; linux-pm@vger.kernel.org
>> Subject: Re: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
>>
>> Hello Wenyou,
>>
>> On Thu, Jan 7, 2016 at 11:13 PM, Wenyou Yang <wenyou.yang@atmel.com>
>> wrote:
>> > This patch adds new driver for Active-semi ACT8945A ActivePath charger
>> > (part of ACT8945A MFD driver) providing power supply class information
>> > to userspace.
>> >
>> > The driver is configured through DTS (battery and system related
>> > settings) and sysfs entries (timers and input over-voltage threshold).
>> >
>> > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
>> > ---
>> >
>>
>> [snip]
>>
>> > +
>> > +static struct platform_driver act8945a_charger_driver = {
>> > + .driver = {
>> > + .name = "act8945a-charger",
>> > + },
>> > + .probe = act8945a_charger_probe,
>> > + .remove = act8945a_charger_remove, };
>> > +
>>
>> The driver is missing an OF match table (and also a platform ID match table if
>> non-DT is supported) so this means that module autoloading will be broken for this
>> driver since the platform uevent function will report a MODALIAS=of:N*T*Cactive-
>> semi,act8945a-charger
>
> This is an MFD device, it compatible is declared in the ../mfd/act8945a.c file.
>
> .of_compatible = "active-semi,act8945a-charger".
>
I know that the device is instantiated from a MFD driver but having a
.of_compatible property in the MFD cell, it only means that the
pdev->dev.of_node field will be filled by the mfd-core [0]. It has
nothing to do with how the platform device is matched to the platform
driver.
The mfd-core calls platform_device_add() to add the device so the
platform bus_type .match function callback [1] will be used to match
the device with the driver and since the driver doesn't have a OF
device ID table to match, it means that this function will fallback to
match the device name with the platform driver name.
Also, you didn't answer my question about module auto-loading. The
platform bus_type .uevent callback calls of_device_uevent_modalias()
and since the device has an .of_node filled by the mfd-core (since you
provided an .of_compatible for the MFD cell), this means that an OF
modalias will be reported but since the driver doesn't have neither a
.of_match_table nor a MODULE_DEVICE_TABLE(of,...) so udev/kmod won't
be able to load the driver's module based on the modalias uevent
reported by the kernel.
> The non-DT is not supported in this patch. So a platform ID match table is not included.
>
Ok, I just asked for completeness.
[0]: http://lxr.free-electrons.com/source/drivers/mfd/mfd-core.c#L177
[1]: http://lxr.free-electrons.com/source/drivers/base/platform.c#L833
[2]: http://lxr.free-electrons.com/source/drivers/base/platform.c#L787
Best regards,
Javier
^ permalink raw reply [flat|nested] 29+ messages in thread* [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
@ 2016-01-12 14:03 ` Javier Martinez Canillas
0 siblings, 0 replies; 29+ messages in thread
From: Javier Martinez Canillas @ 2016-01-12 14:03 UTC (permalink / raw)
To: linux-arm-kernel
Hello Wenyou,
On Tue, Jan 12, 2016 at 5:23 AM, Yang, Wenyou <Wenyou.Yang@atmel.com> wrote:
> Hi Javier,
>
> Thank you for your review.
>
>> -----Original Message-----
>> From: Javier Martinez Canillas [mailto:javier at dowhile0.org]
>> Sent: 2016?1?8? 23:00
>> To: Yang, Wenyou <Wenyou.Yang@atmel.com>
>> Cc: Sebastian Reichel <sre@kernel.org>; Dmitry Eremin-Solenikov
>> <dbaryshkov@gmail.com>; David Woodhouse <dwmw2@infradead.org>; Rob
>> Herring <robh+dt@kernel.org>; Pawel Moll <pawel.moll@arm.com>; Mark
>> Rutland <mark.rutland@arm.com>; Ian Campbell <ijc+devicetree@hellion.org.uk>;
>> Kumar Gala <galak@codeaurora.org>; Ferre, Nicolas
>> <Nicolas.FERRE@atmel.com>; linux-arm-kernel at lists.infradead.org; Linux
>> Kernel <linux-kernel@vger.kernel.org>; linux-pm at vger.kernel.org
>> Subject: Re: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
>>
>> Hello Wenyou,
>>
>> On Thu, Jan 7, 2016 at 11:13 PM, Wenyou Yang <wenyou.yang@atmel.com>
>> wrote:
>> > This patch adds new driver for Active-semi ACT8945A ActivePath charger
>> > (part of ACT8945A MFD driver) providing power supply class information
>> > to userspace.
>> >
>> > The driver is configured through DTS (battery and system related
>> > settings) and sysfs entries (timers and input over-voltage threshold).
>> >
>> > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
>> > ---
>> >
>>
>> [snip]
>>
>> > +
>> > +static struct platform_driver act8945a_charger_driver = {
>> > + .driver = {
>> > + .name = "act8945a-charger",
>> > + },
>> > + .probe = act8945a_charger_probe,
>> > + .remove = act8945a_charger_remove, };
>> > +
>>
>> The driver is missing an OF match table (and also a platform ID match table if
>> non-DT is supported) so this means that module autoloading will be broken for this
>> driver since the platform uevent function will report a MODALIAS=of:N*T*Cactive-
>> semi,act8945a-charger
>
> This is an MFD device, it compatible is declared in the ../mfd/act8945a.c file.
>
> .of_compatible = "active-semi,act8945a-charger".
>
I know that the device is instantiated from a MFD driver but having a
.of_compatible property in the MFD cell, it only means that the
pdev->dev.of_node field will be filled by the mfd-core [0]. It has
nothing to do with how the platform device is matched to the platform
driver.
The mfd-core calls platform_device_add() to add the device so the
platform bus_type .match function callback [1] will be used to match
the device with the driver and since the driver doesn't have a OF
device ID table to match, it means that this function will fallback to
match the device name with the platform driver name.
Also, you didn't answer my question about module auto-loading. The
platform bus_type .uevent callback calls of_device_uevent_modalias()
and since the device has an .of_node filled by the mfd-core (since you
provided an .of_compatible for the MFD cell), this means that an OF
modalias will be reported but since the driver doesn't have neither a
.of_match_table nor a MODULE_DEVICE_TABLE(of,...) so udev/kmod won't
be able to load the driver's module based on the modalias uevent
reported by the kernel.
> The non-DT is not supported in this patch. So a platform ID match table is not included.
>
Ok, I just asked for completeness.
[0]: http://lxr.free-electrons.com/source/drivers/mfd/mfd-core.c#L177
[1]: http://lxr.free-electrons.com/source/drivers/base/platform.c#L833
[2]: http://lxr.free-electrons.com/source/drivers/base/platform.c#L787
Best regards,
Javier
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
@ 2016-01-12 14:03 ` Javier Martinez Canillas
0 siblings, 0 replies; 29+ messages in thread
From: Javier Martinez Canillas @ 2016-01-12 14:03 UTC (permalink / raw)
To: Yang, Wenyou
Cc: Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Ferre, Nicolas, linux-arm-kernel, Linux Kernel, linux-pm
Hello Wenyou,
On Tue, Jan 12, 2016 at 5:23 AM, Yang, Wenyou <Wenyou.Yang@atmel.com> wrote:
> Hi Javier,
>
> Thank you for your review.
>
>> -----Original Message-----
>> From: Javier Martinez Canillas [mailto:javier@dowhile0.org]
>> Sent: 2016年1月8日 23:00
>> To: Yang, Wenyou <Wenyou.Yang@atmel.com>
>> Cc: Sebastian Reichel <sre@kernel.org>; Dmitry Eremin-Solenikov
>> <dbaryshkov@gmail.com>; David Woodhouse <dwmw2@infradead.org>; Rob
>> Herring <robh+dt@kernel.org>; Pawel Moll <pawel.moll@arm.com>; Mark
>> Rutland <mark.rutland@arm.com>; Ian Campbell <ijc+devicetree@hellion.org.uk>;
>> Kumar Gala <galak@codeaurora.org>; Ferre, Nicolas
>> <Nicolas.FERRE@atmel.com>; linux-arm-kernel@lists.infradead.org; Linux
>> Kernel <linux-kernel@vger.kernel.org>; linux-pm@vger.kernel.org
>> Subject: Re: [PATCH 1/2] power: act8945a: add charger driver for ACT8945A
>>
>> Hello Wenyou,
>>
>> On Thu, Jan 7, 2016 at 11:13 PM, Wenyou Yang <wenyou.yang@atmel.com>
>> wrote:
>> > This patch adds new driver for Active-semi ACT8945A ActivePath charger
>> > (part of ACT8945A MFD driver) providing power supply class information
>> > to userspace.
>> >
>> > The driver is configured through DTS (battery and system related
>> > settings) and sysfs entries (timers and input over-voltage threshold).
>> >
>> > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
>> > ---
>> >
>>
>> [snip]
>>
>> > +
>> > +static struct platform_driver act8945a_charger_driver = {
>> > + .driver = {
>> > + .name = "act8945a-charger",
>> > + },
>> > + .probe = act8945a_charger_probe,
>> > + .remove = act8945a_charger_remove, };
>> > +
>>
>> The driver is missing an OF match table (and also a platform ID match table if
>> non-DT is supported) so this means that module autoloading will be broken for this
>> driver since the platform uevent function will report a MODALIAS=of:N*T*Cactive-
>> semi,act8945a-charger
>
> This is an MFD device, it compatible is declared in the ../mfd/act8945a.c file.
>
> .of_compatible = "active-semi,act8945a-charger".
>
I know that the device is instantiated from a MFD driver but having a
.of_compatible property in the MFD cell, it only means that the
pdev->dev.of_node field will be filled by the mfd-core [0]. It has
nothing to do with how the platform device is matched to the platform
driver.
The mfd-core calls platform_device_add() to add the device so the
platform bus_type .match function callback [1] will be used to match
the device with the driver and since the driver doesn't have a OF
device ID table to match, it means that this function will fallback to
match the device name with the platform driver name.
Also, you didn't answer my question about module auto-loading. The
platform bus_type .uevent callback calls of_device_uevent_modalias()
and since the device has an .of_node filled by the mfd-core (since you
provided an .of_compatible for the MFD cell), this means that an OF
modalias will be reported but since the driver doesn't have neither a
.of_match_table nor a MODULE_DEVICE_TABLE(of,...) so udev/kmod won't
be able to load the driver's module based on the modalias uevent
reported by the kernel.
> The non-DT is not supported in this patch. So a platform ID match table is not included.
>
Ok, I just asked for completeness.
[0]: http://lxr.free-electrons.com/source/drivers/mfd/mfd-core.c#L177
[1]: http://lxr.free-electrons.com/source/drivers/base/platform.c#L833
[2]: http://lxr.free-electrons.com/source/drivers/base/platform.c#L787
Best regards,
Javier
^ permalink raw reply [flat|nested] 29+ messages in thread