[PATCH v5 0/2] power: rt5033: Add Richtek RT533 drivers

[PATCH v5 0/2] power: rt5033: Add Richtek RT533 drivers

[Beomho Seo]

This patchset adds driver for Richtek rt5033 chip The chip contains
switching charge mode Li-Ion/Li-Polymer battery charger, fuelgauge.
Additionally, This includes document for device tree of RT5033 device.

RT5033 charger driver and DT documentation merged by Sebastian.
But now, removed both patches from his git tree. Because, It would be nice to
get an ACK of a DT binding maintainer. Could you plase review or some acked?

http://marc.info/?l=linux-pm&m=142195148111292&w=2

RT5033 core driver is applied by Lee Jones.
RT5033 regulator driver is applied by Mark Brown.
RT5033 fuelgauge driver is applied by Sebastian Reichel.

Changes in v5:
- Remove wrong acked-by.

Changes in v4:
- Change power supply type.

Changes in v3:
- Applied one of patchset.
- Add acked-by.

Changes in v2:
- Revise binding documentation.

Beomho Seo (2):
  power: rt5033_charger: Add RT5033 charger device driver
  Documentation: Add documentation for rt5033 multifunction device

 Documentation/devicetree/bindings/mfd/rt5033.txt   |  101 ++++
 .../devicetree/bindings/vendor-prefixes.txt        |    1 +
 drivers/power/Kconfig                              |    8 +
 drivers/power/Makefile                             |    1 +
 drivers/power/rt5033_charger.c                     |  485 ++++++++++++++++++++
 5 files changed, 596 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/rt5033.txt
 create mode 100644 drivers/power/rt5033_charger.c

-- 
1.7.9.5

[PATCH v5 0/2] power: rt5033: Add Richtek RT533 drivers

[Beomho Seo]

This patchset adds driver for Richtek rt5033 chip The chip contains
switching charge mode Li-Ion/Li-Polymer battery charger, fuelgauge.
Additionally, This includes document for device tree of RT5033 device.

RT5033 charger driver and DT documentation merged by Sebastian.
But now, removed both patches from his git tree. Because, It would be nice to
get an ACK of a DT binding maintainer. Could you plase review or some acked?

http://marc.info/?l=linux-pm&m=142195148111292&w=2

RT5033 core driver is applied by Lee Jones.
RT5033 regulator driver is applied by Mark Brown.
RT5033 fuelgauge driver is applied by Sebastian Reichel.

Changes in v5:
- Remove wrong acked-by.

Changes in v4:
- Change power supply type.

Changes in v3:
- Applied one of patchset.
- Add acked-by.

Changes in v2:
- Revise binding documentation.

Beomho Seo (2):
  power: rt5033_charger: Add RT5033 charger device driver
  Documentation: Add documentation for rt5033 multifunction device

 Documentation/devicetree/bindings/mfd/rt5033.txt   |  101 ++++
 .../devicetree/bindings/vendor-prefixes.txt        |    1 +
 drivers/power/Kconfig                              |    8 +
 drivers/power/Makefile                             |    1 +
 drivers/power/rt5033_charger.c                     |  485 ++++++++++++++++++++
 5 files changed, 596 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/rt5033.txt
 create mode 100644 drivers/power/rt5033_charger.c

-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5 1/2] power: rt5033_charger: Add RT5033 charger device driver

[Beomho Seo]

This patch add device driver of Richtek RT5033 PMIC. The driver support
switching charger. rt5033 charger provide three charging mode.
Three charging mode are pre charge mode, fast cahrge mode and constant voltage
mode. They are have vary charge rate, charge parameters. The charge parameters
can be controlled by i2c interface.

Cc: Sebastian Reichel <sre@kernel.org>
Cc: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
---
Changes in v5
- none.
Changes in v4
- Change power supply type to POWER_SUPPLY_TYPE_MAINS.
Changes in v3
Changes in v2
- none

 drivers/power/Kconfig          |    8 +
 drivers/power/Makefile         |    1 +
 drivers/power/rt5033_charger.c |  485 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 494 insertions(+)
 create mode 100644 drivers/power/rt5033_charger.c

diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 27b751b..67e9af7 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -418,6 +418,14 @@ config BATTERY_RT5033
 	  The fuelgauge calculates and determines the battery state of charge
 	  according to battery open circuit voltage.
 
+config CHARGER_RT5033
+	tristate "RT5033 battery charger support"
+	depends on MFD_RT5033
+	help
+	  This adds support for battery charger in Richtek RT5033 PMIC.
+	  The device supports pre-charge mode, fast charge mode and
+	  constant voltage mode.
+
 source "drivers/power/reset/Kconfig"
 
 endif # POWER_SUPPLY
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index 36f9e0d..c5d72e0 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -59,6 +59,7 @@ obj-$(CONFIG_CHARGER_BQ2415X)	+= bq2415x_charger.o
 obj-$(CONFIG_CHARGER_BQ24190)	+= bq24190_charger.o
 obj-$(CONFIG_CHARGER_BQ24735)	+= bq24735-charger.o
 obj-$(CONFIG_POWER_AVS)		+= avs/
+obj-$(CONFIG_POWER_RT5033)	+= rt5033_charger.o
 obj-$(CONFIG_CHARGER_SMB347)	+= smb347-charger.o
 obj-$(CONFIG_CHARGER_TPS65090)	+= tps65090-charger.o
 obj-$(CONFIG_POWER_RESET)	+= reset/
diff --git a/drivers/power/rt5033_charger.c b/drivers/power/rt5033_charger.c
new file mode 100644
index 0000000..7f8f6c3
--- /dev/null
+++ b/drivers/power/rt5033_charger.c
@@ -0,0 +1,485 @@
+/*
+ * Battery charger driver for RT5033
+ *
+ * Copyright (C) 2014 Samsung Electronics, Co., Ltd.
+ * Author: Beomho Seo <beomho.seo@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published bythe Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/mfd/rt5033-private.h>
+#include <linux/mfd/rt5033.h>
+
+static int rt5033_get_charger_state(struct rt5033_charger *charger)
+{
+	struct regmap *regmap = charger->rt5033->regmap;
+	int state = POWER_SUPPLY_STATUS_UNKNOWN;
+	u32 reg_data;
+
+	if (!regmap)
+		return state;
+
+	regmap_read(regmap, RT5033_REG_CHG_STAT, &reg_data);
+
+	switch (reg_data & RT5033_CHG_STAT_MASK) {
+	case RT5033_CHG_STAT_DISCHARGING:
+		state = POWER_SUPPLY_STATUS_DISCHARGING;
+		break;
+	case RT5033_CHG_STAT_CHARGING:
+		state = POWER_SUPPLY_STATUS_CHARGING;
+		break;
+	case RT5033_CHG_STAT_FULL:
+		state = POWER_SUPPLY_STATUS_FULL;
+		break;
+	case RT5033_CHG_STAT_NOT_CHARGING:
+		state = POWER_SUPPLY_STATUS_NOT_CHARGING;
+		break;
+	}
+
+	return state;
+}
+
+static int rt5033_get_charger_type(struct rt5033_charger *charger)
+{
+	struct regmap *regmap = charger->rt5033->regmap;
+	int state = POWER_SUPPLY_CHARGE_TYPE_UNKNOWN;
+	u32 reg_data;
+
+	regmap_read(regmap, RT5033_REG_CHG_STAT, &reg_data);
+
+	switch (reg_data & RT5033_CHG_STAT_TYPE_MASK) {
+	case RT5033_CHG_STAT_TYPE_FAST:
+		state = POWER_SUPPLY_CHARGE_TYPE_FAST;
+		break;
+	case RT5033_CHG_STAT_TYPE_PRE:
+		state = POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
+		break;
+	}
+
+	return state;
+}
+
+static int rt5033_get_charger_current(struct rt5033_charger *charger,
+		enum power_supply_property psp)
+{
+	struct regmap *regmap = charger->rt5033->regmap;
+	unsigned int state, reg_data, data;
+
+	if (psp == POWER_SUPPLY_PROP_CURRENT_MAX)
+		return RT5033_CHG_MAX_CURRENT;
+
+	regmap_read(regmap, RT5033_REG_CHG_CTRL5, &reg_data);
+
+	state = (reg_data >> RT5033_CHGCTRL5_ICHG_SHIFT) & 0xf;
+
+	if (state > RT5033_CHG_MAX_CURRENT)
+		state = RT5033_CHG_MAX_CURRENT;
+
+	data = state * 100 + 700;
+
+	return data;
+}
+
+static int rt5033_get_charge_voltage(struct rt5033_charger *charger,
+		enum power_supply_property psp)
+{
+	struct regmap *regmap = charger->rt5033->regmap;
+	unsigned int state, reg_data, data;
+
+	if (psp == POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX)
+		return RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MAX;
+
+	regmap_read(regmap, RT5033_REG_CHG_CTRL2, &reg_data);
+
+	state = reg_data >> 2;
+
+	data = RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MIN +
+		RT5033_CHARGER_CONST_VOLTAGE_STEP_NUM * state;
+
+	if (data > RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MAX)
+		data = RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MAX;
+
+	return data;
+}
+
+static inline int rt5033_init_const_charge(struct rt5033_charger *psy)
+{
+	struct rt5033_charger_data *chg = psy->chg;
+	unsigned val;
+	int ret;
+	u8 reg_data;
+
+	/* Set Constant voltage mode */
+	if (chg->const_uvolt < RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MIN ||
+		chg->const_uvolt > RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MAX)
+		return -EINVAL;
+
+	if (chg->const_uvolt == RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MIN)
+		reg_data = 0x0;
+	else if (chg->const_uvolt == RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MAX)
+		reg_data = 0xfc;
+	else {
+		val = chg->const_uvolt;
+		val -= RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MIN;
+		val /= RT5033_CHARGER_CONST_VOLTAGE_STEP_NUM;
+		reg_data = val;
+	}
+
+	ret = regmap_update_bits(psy->rt5033->regmap, RT5033_REG_CHG_CTRL2,
+			RT5033_CHGCTRL2_CV_MASK, reg_data << 2);
+	if (ret) {
+		dev_err(psy->dev, "Failed regmap update\n");
+		return -EINVAL;
+	}
+
+	/* Set end of charge current */
+	if (chg->eoc_uamp < RT5033_CHARGER_EOC_MIN ||
+		chg->eoc_uamp > RT5033_CHARGER_EOC_MAX)
+		return -EINVAL;
+
+	if (chg->eoc_uamp == RT5033_CHARGER_EOC_MIN)
+		reg_data = 0x1;
+	else if (chg->eoc_uamp == RT5033_CHARGER_EOC_MAX)
+		reg_data = 0x7;
+	else {
+		val = chg->eoc_uamp;
+		if (val < RT5033_CHARGER_EOC_REF) {
+			val -= RT5033_CHARGER_EOC_MIN;
+			val /= RT5033_CHARGER_EOC_STEP_NUM1;
+			reg_data = 0x01 + val;
+		} else if (val > RT5033_CHARGER_EOC_REF) {
+			val -= RT5033_CHARGER_EOC_REF;
+			val /= RT5033_CHARGER_EOC_STEP_NUM2;
+			reg_data = 0x04 + val;
+		} else {
+			reg_data = 0x04;
+		}
+	}
+
+	ret = regmap_update_bits(psy->rt5033->regmap, RT5033_REG_CHG_CTRL4,
+			RT5033_CHGCTRL4_EOC_MASK, reg_data);
+	if (ret) {
+		dev_err(psy->dev, "Failed regmap update\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static inline int rt5033_init_fast_charge(struct rt5033_charger *psy)
+{
+	struct rt5033_charger_data *chg = psy->chg;
+	int ret;
+	u8 reg_data;
+
+	/* Set limit input current */
+	ret = regmap_update_bits(psy->rt5033->regmap, RT5033_REG_CHG_CTRL1,
+			RT5033_AICR_MODE_MASK, RT5033_AICR_2000_MODE);
+	if (ret) {
+		dev_err(psy->dev, "Failed regmap update\n");
+		return -EINVAL;
+	}
+
+	/* Set internal timer */
+	ret = regmap_update_bits(psy->rt5033->regmap, RT5033_REG_CHG_CTRL3,
+		RT5033_CHGCTRL3_TIMER_MASK | RT5033_CHGCTRL3_TIMER_EN_MASK,
+		RT5033_FAST_CHARGE_TIMER4 | RT5033_INT_TIMER_ENABLE);
+	if (ret) {
+		dev_err(psy->dev, "Failed regmap update\n");
+		return -EINVAL;
+	}
+
+	/* Set fast-charge mode Carging current */
+	if (chg->fast_uamp < RT5033_CHARGER_FAST_CURRENT_MIN ||
+			chg->fast_uamp > RT5033_CHARGER_FAST_CURRENT_MAX)
+		return -EINVAL;
+
+	if (chg->fast_uamp == RT5033_CHARGER_FAST_CURRENT_MIN)
+		reg_data = 0x0;
+	else if (chg->fast_uamp == RT5033_CHARGER_FAST_CURRENT_MAX)
+		reg_data = 0xd0;
+	else {
+		unsigned int val = chg->fast_uamp;
+
+		val -= RT5033_CHARGER_FAST_CURRENT_MIN;
+		val /= RT5033_CHARGER_FAST_CURRENT_STEP_NUM;
+		reg_data = 0x10 + val;
+	}
+
+	ret = regmap_update_bits(psy->rt5033->regmap, RT5033_REG_CHG_CTRL5,
+			RT5033_CHGCTRL5_ICHG_MASK, reg_data);
+	if (ret) {
+		dev_err(psy->dev, "Failed regmap update\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static inline int rt5033_init_pre_charge(struct rt5033_charger *psy)
+{
+	struct rt5033_charger_data *chg = psy->chg;
+	int ret;
+	u8 reg_data;
+
+	/* Set pre-charge threshold voltage */
+	if (chg->pre_uvolt < RT5033_CHARGER_PRE_THRESHOLD_LIMIT_MIN ||
+		chg->pre_uvolt > RT5033_CHARGER_PRE_THRESHOLD_LIMIT_MAX)
+		return -EINVAL;
+
+	if (chg->pre_uvolt == RT5033_CHARGER_PRE_THRESHOLD_LIMIT_MIN)
+		reg_data = 0x00;
+	else if (chg->pre_uvolt == RT5033_CHARGER_PRE_THRESHOLD_LIMIT_MAX)
+		reg_data = 0x0f;
+	else {
+		unsigned int val = chg->pre_uvolt;
+
+		val -= RT5033_CHARGER_PRE_THRESHOLD_LIMIT_MIN;
+		val /= RT5033_CHARGER_PRE_THRESHOLD_STEP_NUM;
+		reg_data = 0x00 + val;
+	}
+
+	ret = regmap_update_bits(psy->rt5033->regmap, RT5033_REG_CHG_CTRL5,
+			RT5033_CHGCTRL5_VPREC_MASK, reg_data);
+	if (ret) {
+		dev_err(psy->dev, "Failed regmap update\n");
+		return -EINVAL;
+	}
+
+	/* Set pre-charge mode charging current */
+	if (chg->pre_uamp < RT5033_CHARGER_PRE_CURRENT_LIMIT_MIN ||
+		chg->pre_uamp > RT5033_CHARGER_PRE_CURRENT_LIMIT_MAX)
+		return -EINVAL;
+
+	if (chg->pre_uamp == RT5033_CHARGER_PRE_CURRENT_LIMIT_MIN)
+		reg_data = 0x00;
+	else if (chg->pre_uamp == RT5033_CHARGER_PRE_CURRENT_LIMIT_MAX)
+		reg_data = 0x18;
+	else {
+		unsigned int val = chg->pre_uamp;
+
+		val -= RT5033_CHARGER_PRE_CURRENT_LIMIT_MIN;
+		val /= RT5033_CHARGER_PRE_CURRENT_STEP_NUM;
+		reg_data = 0x08 + val;
+	}
+
+	ret = regmap_update_bits(psy->rt5033->regmap, RT5033_REG_CHG_CTRL4,
+			RT5033_CHGCTRL4_IPREC_MASK, reg_data);
+	if (ret) {
+		dev_err(psy->dev, "Failed regmap update\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int rt5033_charger_reg_init(struct rt5033_charger *psy)
+{
+	int ret = 0;
+
+	ret = regmap_update_bits(psy->rt5033->regmap, RT5033_REG_CHG_CTRL1,
+			RT5033_CHGCTRL1_MODE_MASK, RT5033_CHARGER_MODE);
+	if (ret) {
+		dev_err(psy->dev, "Failed to update charger mode.\n");
+		return -EINVAL;
+	}
+
+	ret = regmap_update_bits(psy->rt5033->regmap, RT5033_REG_RT_CTRL1,
+			RT5033_RT_CTRL1_UUG_MASK, RT5033_CHARGER_UUG_ENABLE);
+	if (ret) {
+		dev_err(psy->dev, "Failed to update rt ctrl register.\n");
+		return -EINVAL;
+	}
+
+	ret = rt5033_init_pre_charge(psy);
+	if (ret)
+		return ret;
+
+	ret = rt5033_init_fast_charge(psy);
+	if (ret)
+		return ret;
+
+	ret = rt5033_init_const_charge(psy);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(psy->rt5033->regmap, RT5033_REG_CHG_CTRL3,
+			RT5033_CHGCTRL3_CFO_EN_MASK, RT5033_CFO_ENABLE);
+	if (ret) {
+		dev_err(psy->dev, "Failed to set enable.\n");
+		return -EINVAL;
+	}
+
+	ret = regmap_update_bits(psy->rt5033->regmap, RT5033_REG_CHG_CTRL1,
+			RT5033_RT_HZ_MASK, RT5033_CHARGER_HZ_DISABLE);
+	if (ret) {
+		dev_err(psy->dev, "Failed to set high impedance mode.\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static enum power_supply_property rt5033_charger_props[] = {
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_CHARGE_TYPE,
+	POWER_SUPPLY_PROP_CURRENT_NOW,
+	POWER_SUPPLY_PROP_CURRENT_MAX,
+	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
+	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
+	POWER_SUPPLY_PROP_MODEL_NAME,
+	POWER_SUPPLY_PROP_MANUFACTURER,
+};
+
+static int rt5033_charger_get_property(struct power_supply *psy,
+			enum power_supply_property psp,
+			union power_supply_propval *val)
+{
+	struct rt5033_charger *charger = container_of(psy,
+			struct rt5033_charger, psy);
+	int ret = 0;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_STATUS:
+		val->intval = rt5033_get_charger_state(charger);
+		break;
+	case POWER_SUPPLY_PROP_CHARGE_TYPE:
+		val->intval = rt5033_get_charger_type(charger);
+		break;
+	case POWER_SUPPLY_PROP_CURRENT_NOW:
+	case POWER_SUPPLY_PROP_CURRENT_MAX:
+		val->intval = rt5033_get_charger_current(charger, psp);
+		break;
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
+		val->intval = rt5033_get_charge_voltage(charger, psp);
+		break;
+	case POWER_SUPPLY_PROP_MODEL_NAME:
+		val->strval = RT5033_CHARGER_MODEL;
+		break;
+	case POWER_SUPPLY_PROP_MANUFACTURER:
+		val->strval = RT5033_MANUFACTURER;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return ret;
+}
+
+static struct rt5033_charger_data *rt5033_charger_dt_init(
+						struct platform_device *pdev)
+{
+	struct rt5033_charger_data *chg;
+	struct device_node *np = pdev->dev.of_node;
+	int ret;
+
+	if (!np) {
+		dev_err(&pdev->dev, "No charger of_node\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	chg = devm_kzalloc(&pdev->dev, sizeof(*chg), GFP_KERNEL);
+	if (!chg)
+		return ERR_PTR(-ENOMEM);
+
+	ret = of_property_read_u32(np, "richtek,pre-uamp", &chg->pre_uamp);
+	if (ret)
+		return ERR_PTR(ret);
+
+	ret = of_property_read_u32(np, "richtek,pre-threshold-uvolt",
+			&chg->pre_uvolt);
+	if (ret)
+		return ERR_PTR(ret);
+
+	/*
+	 * Charging current is decided by external sensing register and
+	 * regulated voltage. In this driver, external sensing regster value
+	 * is 10 mili ohm
+	 */
+	ret = of_property_read_u32(np, "richtek,fast-uamp", &chg->fast_uamp);
+	if (ret)
+		return ERR_PTR(ret);
+
+	ret = of_property_read_u32(np, "richtek,const-uvolt",
+			&chg->const_uvolt);
+	if (ret)
+		return ERR_PTR(ret);
+
+	ret = of_property_read_u32(np, "richtek,eoc-uamp", &chg->eoc_uamp);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return chg;
+}
+
+static int rt5033_charger_probe(struct platform_device *pdev)
+{
+	struct rt5033_charger *charger;
+	struct rt5033_dev *rt5033 = dev_get_drvdata(pdev->dev.parent);
+	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->rt5033 = rt5033;
+
+	charger->chg = rt5033_charger_dt_init(pdev);
+	if (IS_ERR_OR_NULL(charger->chg))
+		return -ENODEV;
+
+	ret = rt5033_charger_reg_init(charger);
+	if (ret)
+		return ret;
+
+	charger->psy.name = "rt5033-charger",
+	charger->psy.type = POWER_SUPPLY_TYPE_MAINS,
+	charger->psy.properties = rt5033_charger_props,
+	charger->psy.num_properties = ARRAY_SIZE(rt5033_charger_props),
+	charger->psy.get_property = rt5033_charger_get_property,
+
+	ret = power_supply_register(&pdev->dev, &charger->psy);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to register power supply\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int rt5033_charger_remove(struct platform_device *pdev)
+{
+	struct rt5033_charger *charger = platform_get_drvdata(pdev);
+
+	power_supply_unregister(&charger->psy);
+
+	return 0;
+}
+
+static const struct platform_device_id rt5033_charger_id[] = {
+	{ "rt5033-charger", },
+	{ }
+};
+MODULE_DEVICE_TABLE(platform, rt5033_charger_id);
+
+static struct platform_driver rt5033_charger_driver = {
+	.driver = {
+		.name = "rt5033-charger",
+	},
+	.probe = rt5033_charger_probe,
+	.remove = rt5033_charger_remove,
+	.id_table = rt5033_charger_id,
+};
+module_platform_driver(rt5033_charger_driver);
+
+MODULE_DESCRIPTION("Richtek RT5033 charger driver");
+MODULE_AUTHOR("Beomho Seo <beomho.seo@samsung.com>");
+MODULE_LICENSE("GPL");
-- 
1.7.9.5

[PATCH 2/2] Documentation: Add documentation for rt5033 multifunction device

[Beomho Seo]

This patch device tree binding documentation for rt5033 multifunction device.

Cc: Sebastian Reichel <sre@kernel.org>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
---
Changes in v5
- Remove wrong Acked-by.
Changes in v4
- none.
Changes in v3
- Add Acked-by
Changes in v2
- Fix incorrect typo.
- Align -uamp and -uvolt names with regulator binding suffixes.
- Drop incorrect phandle.
- Fix incorrect example.
---
 Documentation/devicetree/bindings/mfd/rt5033.txt   |  101 ++++++++++++++++++++
 .../devicetree/bindings/vendor-prefixes.txt        |    1 +
 2 files changed, 102 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/rt5033.txt

diff --git a/Documentation/devicetree/bindings/mfd/rt5033.txt b/Documentation/devicetree/bindings/mfd/rt5033.txt
new file mode 100644
index 0000000..64b23e8
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/rt5033.txt
@@ -0,0 +1,101 @@
+Richtek RT5033 Power management Integrated Circuit
+
+RT5033 is a Multifunction device which includes battery charger, fuel gauge,
+flash LED current source, LDO and synchronous Buck converter for portable
+applications. It is interfaced to host controller using i2c interface.
+
+Required properties:
+- compatible : Must be "richtek,rt5033"
+- reg : Specifies the i2c slave address of general part.
+- interrupts : This i2c devices has an IRQ line connected to the main SoC.
+- interrupt-parent : The parent interrupt controller.
+
+Optional node:
+Regulators: The regulators of RT5033 have to be instantiated under sub-node
+named "regulators" using the following format.
+
+	regulators {
+		regulator-name {
+			regulator-name = LDO/BUCK
+			regulator subnodes called X, Y and Z
+		};
+	};
+	refer Documentation/devicetree/bindings/regulator/regulator.txt
+
+
+Battery charger: There battery charger of RT5033 have to be instantiated under
+sub-node named "charger" using the following format.
+
+Required properties:
+- compatible : Must be "richtek,rt5033-charger".
+- richtek,pre-uamp : Current of pre-charge mode. The pre-charge current levels
+  are 350 mA to 650 mA programmed by I2C per 100 mA.
+- richtek,fast-uamp : Current of fast-charge mode. The fast-charge current
+  levels are 700 mA to 2000 mA programmed by I2C per 100 mA.
+- richtek,eoc-uamp : This property is end of charge current. Its level 150 mA
+  to 200 mA.
+- richtek,pre-threshold-uvolt : Voltage of threshold pre-charge mode. Battery
+  voltage is below pre-charge threshold voltage, the charger is in pre-charge
+  mode with pre-charge current. Its levels are 2.3 V  to 3.8 V programmed
+  by I2C per 0.1 V.
+- richtek,const-uvolt :  Battery regulation voltage of constant voltage mode.
+  This voltage level 3.65 V to 4.4 V bye I2C per 0.025 V.
+
+	charger {
+		compatible = "richtek,rt5033-charger";
+		richtek,pre-uamp = <350000>;
+		richtek,fast-uamp = <2000000>;
+		richtek,eoc-uamp = <250000>;
+		richtek,pre-threshold-uvolt = <3400000>;
+		richtek,const-uvolt = <4350000>;
+
+	};
+
+
+Fuelgauge: There fuelgauge of RT5033 to be instantiated node named "fuelgauge"
+using the following format.
+
+Required properties:
+- compatible = Must be "richtek,rt5033-battery".
+
+	rt5033@35 {
+		compatible = "richtek,rt5033-battery";
+		interrupt-parent = <&gpx2>;
+		interrupts = <3 0>;
+		reg = <0x35>;
+	};
+
+Example:
+
+		rt5033@34 {
+			compatible = "richtek,rt5033";
+			reg = <0x34>;
+			interrupt-parent = <&gpx1>;
+			interrupts = <5 0>;
+
+			regulators {
+				buck_reg: BUCK {
+					regulator-name = "BUCK";
+					regulator-min-microvolt = <1200000>;
+					regulator-max-microvolt = <1200000>;
+					regulator-always-on;
+				};
+			};
+
+			charger {
+				compatible = "richtek,rt5033-charger";
+				richtek,pre-uamp = <350000>;
+				richtek,fast-uamp = <2000000>;
+				richtek,eoc-uamp = <250000>;
+				richtek,pre-threshold-uvolt = <3400000>;
+				richtek,const-uvolt = <4350000>;
+			};
+
+		};
+
+		rt5033@35 {
+				compatible = "richtek,rt5033-battery";
+				interrupt-parent = <&gpx2>;
+				interrupts = <3 0>;
+				reg = <0x35>;
+		};
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 389ca13..73d235a 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -149,6 +149,7 @@ ralink	Mediatek/Ralink Technology Corp.
 ramtron	Ramtron International
 realtek Realtek Semiconductor Corp.
 renesas	Renesas Electronics Corporation
+richtek	Richtek Technology Corporation
 ricoh	Ricoh Co. Ltd.
 rockchip	Fuzhou Rockchip Electronics Co., Ltd
 samsung	Samsung Semiconductor
-- 
1.7.9.5

Re: [PATCH v5 1/2] power: rt5033_charger: Add RT5033 charger device driver

[Sebastian Reichel]

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

Hi Beomho,

On Mon, Mar 09, 2015 at 10:23:10AM +0900, Beomho Seo wrote:
> This patch add device driver of Richtek RT5033 PMIC. The driver support
> switching charger. rt5033 charger provide three charging mode.
> Three charging mode are pre charge mode, fast cahrge mode and constant voltage
> mode. They are have vary charge rate, charge parameters. The charge parameters
> can be controlled by i2c interface.

Driver looks mostly ok, but I have some comments [inline].

> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
> Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
> Changes in v5
> - none.
> Changes in v4
> - Change power supply type to POWER_SUPPLY_TYPE_MAINS.
> Changes in v3
> Changes in v2
> - none
> 
>  drivers/power/Kconfig          |    8 +
>  drivers/power/Makefile         |    1 +
>  drivers/power/rt5033_charger.c |  485 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 494 insertions(+)
>  create mode 100644 drivers/power/rt5033_charger.c
> 
> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
> index 27b751b..67e9af7 100644
> --- a/drivers/power/Kconfig
> +++ b/drivers/power/Kconfig
> @@ -418,6 +418,14 @@ config BATTERY_RT5033
>  	  The fuelgauge calculates and determines the battery state of charge
>  	  according to battery open circuit voltage.
>  
> +config CHARGER_RT5033
> +	tristate "RT5033 battery charger support"
> +	depends on MFD_RT5033
> +	help
> +	  This adds support for battery charger in Richtek RT5033 PMIC.
> +	  The device supports pre-charge mode, fast charge mode and
> +	  constant voltage mode.
> +
>  source "drivers/power/reset/Kconfig"
>  
>  endif # POWER_SUPPLY
> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
> index 36f9e0d..c5d72e0 100644
> --- a/drivers/power/Makefile
> +++ b/drivers/power/Makefile
> @@ -59,6 +59,7 @@ obj-$(CONFIG_CHARGER_BQ2415X)	+= bq2415x_charger.o
>  obj-$(CONFIG_CHARGER_BQ24190)	+= bq24190_charger.o
>  obj-$(CONFIG_CHARGER_BQ24735)	+= bq24735-charger.o
>  obj-$(CONFIG_POWER_AVS)		+= avs/
> +obj-$(CONFIG_POWER_RT5033)	+= rt5033_charger.o

this should be 

obj-$(CONFIG_CHARGER_RT5033) += rt5033_charger.o

according to your Kconfig patch. How did you test it actually?

>  obj-$(CONFIG_CHARGER_SMB347)	+= smb347-charger.o
>  obj-$(CONFIG_CHARGER_TPS65090)	+= tps65090-charger.o
>  obj-$(CONFIG_POWER_RESET)	+= reset/
> diff --git a/drivers/power/rt5033_charger.c b/drivers/power/rt5033_charger.c
> new file mode 100644
> index 0000000..7f8f6c3
> --- /dev/null
> +++ b/drivers/power/rt5033_charger.c
>
> [...]
>
> +static int rt5033_get_charger_current(struct rt5033_charger *charger,
> +		enum power_supply_property psp)
> +{
> +	struct regmap *regmap = charger->rt5033->regmap;
> +	unsigned int state, reg_data, data;
> +
> +	if (psp == POWER_SUPPLY_PROP_CURRENT_MAX)
> +		return RT5033_CHG_MAX_CURRENT;

drop this psp check and the psp parameter to this function, so that
the function only takes care of the current current.

> +	regmap_read(regmap, RT5033_REG_CHG_CTRL5, &reg_data);
> +
> +	state = (reg_data >> RT5033_CHGCTRL5_ICHG_SHIFT) & 0xf;
> +
> +	if (state > RT5033_CHG_MAX_CURRENT)
> +		state = RT5033_CHG_MAX_CURRENT;
> +
> +	data = state * 100 + 700;
> +
> +	return data;
> +}
> +
> +static int rt5033_get_charge_voltage(struct rt5033_charger *charger,
> +		enum power_supply_property psp)
> +{
> +	struct regmap *regmap = charger->rt5033->regmap;
> +	unsigned int state, reg_data, data;
> +
> +	if (psp == POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX)
> +		return RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MAX;

drop this psp check and the psp parameter to this function, so that
the function only takes care of the current voltage.

> +	regmap_read(regmap, RT5033_REG_CHG_CTRL2, &reg_data);
> +
> +	state = reg_data >> 2;
> +
> +	data = RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MIN +
> +		RT5033_CHARGER_CONST_VOLTAGE_STEP_NUM * state;
> +
> +	if (data > RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MAX)
> +		data = RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MAX;
> +
> +	return data;
> +}
>
> [...]
>
> +
> +static enum power_supply_property rt5033_charger_props[] = {
> +	POWER_SUPPLY_PROP_STATUS,
> +	POWER_SUPPLY_PROP_CHARGE_TYPE,
> +	POWER_SUPPLY_PROP_CURRENT_NOW,
> +	POWER_SUPPLY_PROP_CURRENT_MAX,
> +	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
> +	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
> +	POWER_SUPPLY_PROP_MODEL_NAME,
> +	POWER_SUPPLY_PROP_MANUFACTURER,
> +};
> +
> +static int rt5033_charger_get_property(struct power_supply *psy,
> +			enum power_supply_property psp,
> +			union power_supply_propval *val)
> +{
> +	struct rt5033_charger *charger = container_of(psy,
> +			struct rt5033_charger, psy);
> +	int ret = 0;
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_STATUS:
> +		val->intval = rt5033_get_charger_state(charger);
> +		break;
> +	case POWER_SUPPLY_PROP_CHARGE_TYPE:
> +		val->intval = rt5033_get_charger_type(charger);
> +		break;
> +	case POWER_SUPPLY_PROP_CURRENT_NOW:
> +	case POWER_SUPPLY_PROP_CURRENT_MAX:
> +		val->intval = rt5033_get_charger_current(charger, psp);
> +		break;

remove the special handling of POWER_SUPPLY_PROP_CURRENT_MAX in 
rt5033_get_charger_current() and do it like this:

case POWER_SUPPLY_PROP_CURRENT_NOW:
    val->intval = rt5033_get_charger_current(charger);
    break;
case POWER_SUPPLY_PROP_CURRENT_MAX:
    val->intval = RT5033_CHG_MAX_CURRENT;
    break;

> +	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
> +	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
> +		val->intval = rt5033_get_charge_voltage(charger, psp);
> +		break;

same as current handling:

case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
    val->intval = rt5033_get_charge_voltage(charger);
    break;
case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
    val->intval = RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MAX;
    break;

> +	case POWER_SUPPLY_PROP_MODEL_NAME:
> +		val->strval = RT5033_CHARGER_MODEL;
> +		break;
> +	case POWER_SUPPLY_PROP_MANUFACTURER:
> +		val->strval = RT5033_MANUFACTURER;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return ret;
> +}
>
> [...]
>
> +static int rt5033_charger_probe(struct platform_device *pdev)
> +{
> +	struct rt5033_charger *charger;
> +	struct rt5033_dev *rt5033 = dev_get_drvdata(pdev->dev.parent);
> +	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->rt5033 = rt5033;
> +
> +	charger->chg = rt5033_charger_dt_init(pdev);
> +	if (IS_ERR_OR_NULL(charger->chg))
> +		return -ENODEV;
> +
> +	ret = rt5033_charger_reg_init(charger);
> +	if (ret)
> +		return ret;
> +
> +	charger->psy.name = "rt5033-charger",
> +	charger->psy.type = POWER_SUPPLY_TYPE_MAINS,
> +	charger->psy.properties = rt5033_charger_props,
> +	charger->psy.num_properties = ARRAY_SIZE(rt5033_charger_props),
> +	charger->psy.get_property = rt5033_charger_get_property,
> +
> +	ret = power_supply_register(&pdev->dev, &charger->psy);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to register power supply\n");
> +		return ret;
> +	}

We have devm_power_supply_register() now, which would result in
complete removal of the rt5033_charger_remove() function :)

> +	return 0;
> +}
> +
> +static int rt5033_charger_remove(struct platform_device *pdev)
> +{
> +	struct rt5033_charger *charger = platform_get_drvdata(pdev);
> +
> +	power_supply_unregister(&charger->psy);
> +
> +	return 0;
> +}
> +
> +static const struct platform_device_id rt5033_charger_id[] = {
> +	{ "rt5033-charger", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(platform, rt5033_charger_id);
> +
> +static struct platform_driver rt5033_charger_driver = {
> +	.driver = {
> +		.name = "rt5033-charger",
> +	},
> +	.probe = rt5033_charger_probe,
> +	.remove = rt5033_charger_remove,
> +	.id_table = rt5033_charger_id,
> +};
> +module_platform_driver(rt5033_charger_driver);
> +
> +MODULE_DESCRIPTION("Richtek RT5033 charger driver");
> +MODULE_AUTHOR("Beomho Seo <beomho.seo@samsung.com>");
> +MODULE_LICENSE("GPL");

This should be 

MODULE_LICENSE("GPL v2");

-- Sebastian

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

Re: [PATCH v5 1/2] power: rt5033_charger: Add RT5033 charger device driver

[Beomho Seo]

On 03/09/2015 10:50 AM, Sebastian Reichel wrote:
> Hi Beomho,
> 
> On Mon, Mar 09, 2015 at 10:23:10AM +0900, Beomho Seo wrote:
>> This patch add device driver of Richtek RT5033 PMIC. The driver support
>> switching charger. rt5033 charger provide three charging mode.
>> Three charging mode are pre charge mode, fast cahrge mode and constant voltage
>> mode. They are have vary charge rate, charge parameters. The charge parameters
>> can be controlled by i2c interface.
> 
> Driver looks mostly ok, but I have some comments [inline].
> 
>> Cc: Sebastian Reichel <sre@kernel.org>
>> Cc: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
>> Cc: David Woodhouse <dwmw2@infradead.org>
>> Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
>> Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
>> ---
>> Changes in v5
>> - none.
>> Changes in v4
>> - Change power supply type to POWER_SUPPLY_TYPE_MAINS.
>> Changes in v3
>> Changes in v2
>> - none
>>
>>  drivers/power/Kconfig          |    8 +
>>  drivers/power/Makefile         |    1 +
>>  drivers/power/rt5033_charger.c |  485 ++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 494 insertions(+)
>>  create mode 100644 drivers/power/rt5033_charger.c
>>
>> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
>> index 27b751b..67e9af7 100644
>> --- a/drivers/power/Kconfig
>> +++ b/drivers/power/Kconfig
>> @@ -418,6 +418,14 @@ config BATTERY_RT5033
>>  	  The fuelgauge calculates and determines the battery state of charge
>>  	  according to battery open circuit voltage.
>>  
>> +config CHARGER_RT5033
>> +	tristate "RT5033 battery charger support"
>> +	depends on MFD_RT5033
>> +	help
>> +	  This adds support for battery charger in Richtek RT5033 PMIC.
>> +	  The device supports pre-charge mode, fast charge mode and
>> +	  constant voltage mode.
>> +
>>  source "drivers/power/reset/Kconfig"
>>  
>>  endif # POWER_SUPPLY
>> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
>> index 36f9e0d..c5d72e0 100644
>> --- a/drivers/power/Makefile
>> +++ b/drivers/power/Makefile
>> @@ -59,6 +59,7 @@ obj-$(CONFIG_CHARGER_BQ2415X)	+= bq2415x_charger.o
>>  obj-$(CONFIG_CHARGER_BQ24190)	+= bq24190_charger.o
>>  obj-$(CONFIG_CHARGER_BQ24735)	+= bq24735-charger.o
>>  obj-$(CONFIG_POWER_AVS)		+= avs/
>> +obj-$(CONFIG_POWER_RT5033)	+= rt5033_charger.o
> 
> this should be 
> 
> obj-$(CONFIG_CHARGER_RT5033) += rt5033_charger.o
> 
> according to your Kconfig patch. How did you test it actually?
> 

Sorry, My mistake. This patch have been tested on my board.
I will double-check before send patch set.

>>  obj-$(CONFIG_CHARGER_SMB347)	+= smb347-charger.o
>>  obj-$(CONFIG_CHARGER_TPS65090)	+= tps65090-charger.o
>>  obj-$(CONFIG_POWER_RESET)	+= reset/
>> diff --git a/drivers/power/rt5033_charger.c b/drivers/power/rt5033_charger.c
>> new file mode 100644
>> index 0000000..7f8f6c3
>> --- /dev/null
>> +++ b/drivers/power/rt5033_charger.c
>>
>> [...]
>>
>> +static int rt5033_get_charger_current(struct rt5033_charger *charger,
>> +		enum power_supply_property psp)
>> +{
>> +	struct regmap *regmap = charger->rt5033->regmap;
>> +	unsigned int state, reg_data, data;
>> +
>> +	if (psp == POWER_SUPPLY_PROP_CURRENT_MAX)
>> +		return RT5033_CHG_MAX_CURRENT;
> 
> drop this psp check and the psp parameter to this function, so that
> the function only takes care of the current current.
> 

OK. I will remove above lines.

>> +	regmap_read(regmap, RT5033_REG_CHG_CTRL5, &reg_data);
>> +
>> +	state = (reg_data >> RT5033_CHGCTRL5_ICHG_SHIFT) & 0xf;
>> +
>> +	if (state > RT5033_CHG_MAX_CURRENT)
>> +		state = RT5033_CHG_MAX_CURRENT;
>> +
>> +	data = state * 100 + 700;
>> +
>> +	return data;
>> +}
>> +
>> +static int rt5033_get_charge_voltage(struct rt5033_charger *charger,
>> +		enum power_supply_property psp)
>> +{
>> +	struct regmap *regmap = charger->rt5033->regmap;
>> +	unsigned int state, reg_data, data;
>> +
>> +	if (psp == POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX)
>> +		return RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MAX;
> 
> drop this psp check and the psp parameter to this function, so that
> the function only takes care of the current voltage.
> 

OK. I will remove above lines.

>> +	regmap_read(regmap, RT5033_REG_CHG_CTRL2, &reg_data);
>> +
>> +	state = reg_data >> 2;
>> +
>> +	data = RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MIN +
>> +		RT5033_CHARGER_CONST_VOLTAGE_STEP_NUM * state;
>> +
>> +	if (data > RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MAX)
>> +		data = RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MAX;
>> +
>> +	return data;
>> +}
>>
>> [...]
>>
>> +
>> +static enum power_supply_property rt5033_charger_props[] = {
>> +	POWER_SUPPLY_PROP_STATUS,
>> +	POWER_SUPPLY_PROP_CHARGE_TYPE,
>> +	POWER_SUPPLY_PROP_CURRENT_NOW,
>> +	POWER_SUPPLY_PROP_CURRENT_MAX,
>> +	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
>> +	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
>> +	POWER_SUPPLY_PROP_MODEL_NAME,
>> +	POWER_SUPPLY_PROP_MANUFACTURER,
>> +};
>> +
>> +static int rt5033_charger_get_property(struct power_supply *psy,
>> +			enum power_supply_property psp,
>> +			union power_supply_propval *val)
>> +{
>> +	struct rt5033_charger *charger = container_of(psy,
>> +			struct rt5033_charger, psy);
>> +	int ret = 0;
>> +
>> +	switch (psp) {
>> +	case POWER_SUPPLY_PROP_STATUS:
>> +		val->intval = rt5033_get_charger_state(charger);
>> +		break;
>> +	case POWER_SUPPLY_PROP_CHARGE_TYPE:
>> +		val->intval = rt5033_get_charger_type(charger);
>> +		break;
>> +	case POWER_SUPPLY_PROP_CURRENT_NOW:
>> +	case POWER_SUPPLY_PROP_CURRENT_MAX:
>> +		val->intval = rt5033_get_charger_current(charger, psp);
>> +		break;
> 
> remove the special handling of POWER_SUPPLY_PROP_CURRENT_MAX in 
> rt5033_get_charger_current() and do it like this:
> 
> case POWER_SUPPLY_PROP_CURRENT_NOW:
>     val->intval = rt5033_get_charger_current(charger);
>     break;
> case POWER_SUPPLY_PROP_CURRENT_MAX:
>     val->intval = RT5033_CHG_MAX_CURRENT;
>     break;
> 

OK. I will change comply with your comment.

>> +	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
>> +	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
>> +		val->intval = rt5033_get_charge_voltage(charger, psp);
>> +		break;
> 
> same as current handling:
> 
> case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
>     val->intval = rt5033_get_charge_voltage(charger);
>     break;
> case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
>     val->intval = RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MAX;
>     break;
> 

Ditto.

>> +	case POWER_SUPPLY_PROP_MODEL_NAME:
>> +		val->strval = RT5033_CHARGER_MODEL;
>> +		break;
>> +	case POWER_SUPPLY_PROP_MANUFACTURER:
>> +		val->strval = RT5033_MANUFACTURER;
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	return ret;
>> +}
>>
>> [...]
>>
>> +static int rt5033_charger_probe(struct platform_device *pdev)
>> +{
>> +	struct rt5033_charger *charger;
>> +	struct rt5033_dev *rt5033 = dev_get_drvdata(pdev->dev.parent);
>> +	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->rt5033 = rt5033;
>> +
>> +	charger->chg = rt5033_charger_dt_init(pdev);
>> +	if (IS_ERR_OR_NULL(charger->chg))
>> +		return -ENODEV;
>> +
>> +	ret = rt5033_charger_reg_init(charger);
>> +	if (ret)
>> +		return ret;
>> +
>> +	charger->psy.name = "rt5033-charger",
>> +	charger->psy.type = POWER_SUPPLY_TYPE_MAINS,
>> +	charger->psy.properties = rt5033_charger_props,
>> +	charger->psy.num_properties = ARRAY_SIZE(rt5033_charger_props),
>> +	charger->psy.get_property = rt5033_charger_get_property,
>> +
>> +	ret = power_supply_register(&pdev->dev, &charger->psy);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "Failed to register power supply\n");
>> +		return ret;
>> +	}
> 
> We have devm_power_supply_register() now, which would result in
> complete removal of the rt5033_charger_remove() function :)
> 

OK, I will change comply with your comment.

>> +	return 0;
>> +}
>> +
>> +static int rt5033_charger_remove(struct platform_device *pdev)
>> +{
>> +	struct rt5033_charger *charger = platform_get_drvdata(pdev);
>> +
>> +	power_supply_unregister(&charger->psy);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct platform_device_id rt5033_charger_id[] = {
>> +	{ "rt5033-charger", },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(platform, rt5033_charger_id);
>> +
>> +static struct platform_driver rt5033_charger_driver = {
>> +	.driver = {
>> +		.name = "rt5033-charger",
>> +	},
>> +	.probe = rt5033_charger_probe,
>> +	.remove = rt5033_charger_remove,
>> +	.id_table = rt5033_charger_id,
>> +};
>> +module_platform_driver(rt5033_charger_driver);
>> +
>> +MODULE_DESCRIPTION("Richtek RT5033 charger driver");
>> +MODULE_AUTHOR("Beomho Seo <beomho.seo@samsung.com>");
>> +MODULE_LICENSE("GPL");
> 
> This should be 
> 
> MODULE_LICENSE("GPL v2");
> 
> -- Sebastian
> 

Thanks your comment.
I will change this patch as soon as I possibly can.

Best regards,
Beomho Seo

Re: [PATCH v5 1/2] power: rt5033_charger: Add RT5033 charger device driver

[Beomho Seo]

On 03/09/2015 12:46 PM, Beomho Seo wrote:
> On 03/09/2015 10:50 AM, Sebastian Reichel wrote:
>> Hi Beomho,
>>
>> On Mon, Mar 09, 2015 at 10:23:10AM +0900, Beomho Seo wrote:
>>> This patch add device driver of Richtek RT5033 PMIC. The driver support
>>> switching charger. rt5033 charger provide three charging mode.
>>> Three charging mode are pre charge mode, fast cahrge mode and constant voltage
>>> mode. They are have vary charge rate, charge parameters. The charge parameters
>>> can be controlled by i2c interface.
>>
>> Driver looks mostly ok, but I have some comments [inline].
>>
>>> Cc: Sebastian Reichel <sre@kernel.org>
>>> Cc: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
>>> Cc: David Woodhouse <dwmw2@infradead.org>
>>> Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
>>> Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
>>> ---
>>> Changes in v5
>>> - none.
>>> Changes in v4
>>> - Change power supply type to POWER_SUPPLY_TYPE_MAINS.
>>> Changes in v3
>>> Changes in v2
>>> - none
>>>
>>>  drivers/power/Kconfig          |    8 +
>>>  drivers/power/Makefile         |    1 +
>>>  drivers/power/rt5033_charger.c |  485 ++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 494 insertions(+)
>>>  create mode 100644 drivers/power/rt5033_charger.c
>>>
>>> diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
>>> index 27b751b..67e9af7 100644
>>> --- a/drivers/power/Kconfig
>>> +++ b/drivers/power/Kconfig
>>> @@ -418,6 +418,14 @@ config BATTERY_RT5033
>>>  	  The fuelgauge calculates and determines the battery state of charge
>>>  	  according to battery open circuit voltage.
>>>  
>>> +config CHARGER_RT5033
>>> +	tristate "RT5033 battery charger support"
>>> +	depends on MFD_RT5033
>>> +	help
>>> +	  This adds support for battery charger in Richtek RT5033 PMIC.
>>> +	  The device supports pre-charge mode, fast charge mode and
>>> +	  constant voltage mode.
>>> +
>>>  source "drivers/power/reset/Kconfig"
>>>  
>>>  endif # POWER_SUPPLY
>>> diff --git a/drivers/power/Makefile b/drivers/power/Makefile
>>> index 36f9e0d..c5d72e0 100644
>>> --- a/drivers/power/Makefile
>>> +++ b/drivers/power/Makefile
>>> @@ -59,6 +59,7 @@ obj-$(CONFIG_CHARGER_BQ2415X)	+= bq2415x_charger.o
>>>  obj-$(CONFIG_CHARGER_BQ24190)	+= bq24190_charger.o
>>>  obj-$(CONFIG_CHARGER_BQ24735)	+= bq24735-charger.o
>>>  obj-$(CONFIG_POWER_AVS)		+= avs/
>>> +obj-$(CONFIG_POWER_RT5033)	+= rt5033_charger.o
>>
>> this should be 
>>
>> obj-$(CONFIG_CHARGER_RT5033) += rt5033_charger.o
>>
>> according to your Kconfig patch. How did you test it actually?
>>
> 
> Sorry, My mistake. This patch have been tested on my board.
> I will double-check before send patch set.
> 
>>>  obj-$(CONFIG_CHARGER_SMB347)	+= smb347-charger.o
>>>  obj-$(CONFIG_CHARGER_TPS65090)	+= tps65090-charger.o
>>>  obj-$(CONFIG_POWER_RESET)	+= reset/
>>> diff --git a/drivers/power/rt5033_charger.c b/drivers/power/rt5033_charger.c
>>> new file mode 100644
>>> index 0000000..7f8f6c3
>>> --- /dev/null
>>> +++ b/drivers/power/rt5033_charger.c
>>>
>>> [...]
>>>
>>> +static int rt5033_get_charger_current(struct rt5033_charger *charger,
>>> +		enum power_supply_property psp)
>>> +{
>>> +	struct regmap *regmap = charger->rt5033->regmap;
>>> +	unsigned int state, reg_data, data;
>>> +
>>> +	if (psp == POWER_SUPPLY_PROP_CURRENT_MAX)
>>> +		return RT5033_CHG_MAX_CURRENT;
>>
>> drop this psp check and the psp parameter to this function, so that
>> the function only takes care of the current current.
>>
> 
> OK. I will remove above lines.
> 
>>> +	regmap_read(regmap, RT5033_REG_CHG_CTRL5, &reg_data);
>>> +
>>> +	state = (reg_data >> RT5033_CHGCTRL5_ICHG_SHIFT) & 0xf;
>>> +
>>> +	if (state > RT5033_CHG_MAX_CURRENT)
>>> +		state = RT5033_CHG_MAX_CURRENT;
>>> +
>>> +	data = state * 100 + 700;
>>> +
>>> +	return data;
>>> +}
>>> +
>>> +static int rt5033_get_charge_voltage(struct rt5033_charger *charger,
>>> +		enum power_supply_property psp)
>>> +{
>>> +	struct regmap *regmap = charger->rt5033->regmap;
>>> +	unsigned int state, reg_data, data;
>>> +
>>> +	if (psp == POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX)
>>> +		return RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MAX;
>>
>> drop this psp check and the psp parameter to this function, so that
>> the function only takes care of the current voltage.
>>
> 
> OK. I will remove above lines.
> 
>>> +	regmap_read(regmap, RT5033_REG_CHG_CTRL2, &reg_data);
>>> +
>>> +	state = reg_data >> 2;
>>> +
>>> +	data = RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MIN +
>>> +		RT5033_CHARGER_CONST_VOLTAGE_STEP_NUM * state;
>>> +
>>> +	if (data > RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MAX)
>>> +		data = RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MAX;
>>> +
>>> +	return data;
>>> +}
>>>
>>> [...]
>>>
>>> +
>>> +static enum power_supply_property rt5033_charger_props[] = {
>>> +	POWER_SUPPLY_PROP_STATUS,
>>> +	POWER_SUPPLY_PROP_CHARGE_TYPE,
>>> +	POWER_SUPPLY_PROP_CURRENT_NOW,
>>> +	POWER_SUPPLY_PROP_CURRENT_MAX,
>>> +	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE,
>>> +	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
>>> +	POWER_SUPPLY_PROP_MODEL_NAME,
>>> +	POWER_SUPPLY_PROP_MANUFACTURER,
>>> +};
>>> +
>>> +static int rt5033_charger_get_property(struct power_supply *psy,
>>> +			enum power_supply_property psp,
>>> +			union power_supply_propval *val)
>>> +{
>>> +	struct rt5033_charger *charger = container_of(psy,
>>> +			struct rt5033_charger, psy);
>>> +	int ret = 0;
>>> +
>>> +	switch (psp) {
>>> +	case POWER_SUPPLY_PROP_STATUS:
>>> +		val->intval = rt5033_get_charger_state(charger);
>>> +		break;
>>> +	case POWER_SUPPLY_PROP_CHARGE_TYPE:
>>> +		val->intval = rt5033_get_charger_type(charger);
>>> +		break;
>>> +	case POWER_SUPPLY_PROP_CURRENT_NOW:
>>> +	case POWER_SUPPLY_PROP_CURRENT_MAX:
>>> +		val->intval = rt5033_get_charger_current(charger, psp);
>>> +		break;
>>
>> remove the special handling of POWER_SUPPLY_PROP_CURRENT_MAX in 
>> rt5033_get_charger_current() and do it like this:
>>
>> case POWER_SUPPLY_PROP_CURRENT_NOW:
>>     val->intval = rt5033_get_charger_current(charger);
>>     break;
>> case POWER_SUPPLY_PROP_CURRENT_MAX:
>>     val->intval = RT5033_CHG_MAX_CURRENT;
>>     break;
>>
> 
> OK. I will change comply with your comment.
> 

RT5033_CHG_MAX_CURRENT is register value(hex).
So It is better:

case POWER_SUPPLY_PROP_CURRENT_MAX:
	val->intval = RT5033_CHARGER_FAST_CURRENT_MAX;

And then I will fix rt5033_get_charger_current function more readable.

>>> +	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
>>> +	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
>>> +		val->intval = rt5033_get_charge_voltage(charger, psp);
>>> +		break;
>>
>> same as current handling:
>>
>> case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
>>     val->intval = rt5033_get_charge_voltage(charger);
>>     break;
>> case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
>>     val->intval = RT5033_CHARGER_CONST_VOLTAGE_LIMIT_MAX;
>>     break;
>>
> 
> Ditto.
> 
>>> +	case POWER_SUPPLY_PROP_MODEL_NAME:
>>> +		val->strval = RT5033_CHARGER_MODEL;
>>> +		break;
>>> +	case POWER_SUPPLY_PROP_MANUFACTURER:
>>> +		val->strval = RT5033_MANUFACTURER;
>>> +		break;
>>> +	default:
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	return ret;
>>> +}
>>>
>>> [...]
>>>
>>> +static int rt5033_charger_probe(struct platform_device *pdev)
>>> +{
>>> +	struct rt5033_charger *charger;
>>> +	struct rt5033_dev *rt5033 = dev_get_drvdata(pdev->dev.parent);
>>> +	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->rt5033 = rt5033;
>>> +
>>> +	charger->chg = rt5033_charger_dt_init(pdev);
>>> +	if (IS_ERR_OR_NULL(charger->chg))
>>> +		return -ENODEV;
>>> +
>>> +	ret = rt5033_charger_reg_init(charger);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	charger->psy.name = "rt5033-charger",
>>> +	charger->psy.type = POWER_SUPPLY_TYPE_MAINS,
>>> +	charger->psy.properties = rt5033_charger_props,
>>> +	charger->psy.num_properties = ARRAY_SIZE(rt5033_charger_props),
>>> +	charger->psy.get_property = rt5033_charger_get_property,
>>> +
>>> +	ret = power_supply_register(&pdev->dev, &charger->psy);
>>> +	if (ret) {
>>> +		dev_err(&pdev->dev, "Failed to register power supply\n");
>>> +		return ret;
>>> +	}
>>
>> We have devm_power_supply_register() now, which would result in
>> complete removal of the rt5033_charger_remove() function :)
>>
> 
> OK, I will change comply with your comment.
> 
>>> +	return 0;
>>> +}
>>> +
>>> +static int rt5033_charger_remove(struct platform_device *pdev)
>>> +{
>>> +	struct rt5033_charger *charger = platform_get_drvdata(pdev);
>>> +
>>> +	power_supply_unregister(&charger->psy);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static const struct platform_device_id rt5033_charger_id[] = {
>>> +	{ "rt5033-charger", },
>>> +	{ }
>>> +};
>>> +MODULE_DEVICE_TABLE(platform, rt5033_charger_id);
>>> +
>>> +static struct platform_driver rt5033_charger_driver = {
>>> +	.driver = {
>>> +		.name = "rt5033-charger",
>>> +	},
>>> +	.probe = rt5033_charger_probe,
>>> +	.remove = rt5033_charger_remove,
>>> +	.id_table = rt5033_charger_id,
>>> +};
>>> +module_platform_driver(rt5033_charger_driver);
>>> +
>>> +MODULE_DESCRIPTION("Richtek RT5033 charger driver");
>>> +MODULE_AUTHOR("Beomho Seo <beomho.seo@samsung.com>");
>>> +MODULE_LICENSE("GPL");
>>
>> This should be 
>>
>> MODULE_LICENSE("GPL v2");
>>
>> -- Sebastian
>>
> 
> Thanks your comment.
> I will change this patch as soon as I possibly can.
> 
> Best regards,
> Beomho Seo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 2/2] Documentation: Add documentation for rt5033 multifunction device

[Lee Jones]

FAO Mark,

> This patch device tree binding documentation for rt5033 multifunction device.
> 
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Signed-off-by: Beomho Seo <beomho.seo@samsung.com>
> Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
> Changes in v5
> - Remove wrong Acked-by.
> Changes in v4
> - none.
> Changes in v3
> - Add Acked-by
> Changes in v2
> - Fix incorrect typo.
> - Align -uamp and -uvolt names with regulator binding suffixes.
> - Drop incorrect phandle.
> - Fix incorrect example.
> ---
>  Documentation/devicetree/bindings/mfd/rt5033.txt   |  101 ++++++++++++++++++++

[...]

>  .../devicetree/bindings/vendor-prefixes.txt        |    1 +

This change will be low resistance.  I suggest you separate it out
into another patch.

>  2 files changed, 102 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/rt5033.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/rt5033.txt b/Documentation/devicetree/bindings/mfd/rt5033.txt
> new file mode 100644
> index 0000000..64b23e8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/rt5033.txt
> @@ -0,0 +1,101 @@
> +Richtek RT5033 Power management Integrated Circuit
> +
> +RT5033 is a Multifunction device which includes battery charger, fuel gauge,
> +flash LED current source, LDO and synchronous Buck converter for portable
> +applications. It is interfaced to host controller using i2c interface.
> +
> +Required properties:
> +- compatible : Must be "richtek,rt5033"
> +- reg : Specifies the i2c slave address of general part.
> +- interrupts : This i2c devices has an IRQ line connected to the main SoC.
> +- interrupt-parent : The parent interrupt controller.
> +
> +Optional node:
> +Regulators: The regulators of RT5033 have to be instantiated under sub-node
> +named "regulators" using the following format.
> +
> +	regulators {
> +		regulator-name {
> +			regulator-name = LDO/BUCK
> +			regulator subnodes called X, Y and Z
> +		};
> +	};
> +	refer Documentation/devicetree/bindings/regulator/regulator.txt
> +
> +
> +Battery charger: There battery charger of RT5033 have to be instantiated under
> +sub-node named "charger" using the following format.
> +
> +Required properties:
> +- compatible : Must be "richtek,rt5033-charger".
> +- richtek,pre-uamp : Current of pre-charge mode. The pre-charge current levels
> +  are 350 mA to 650 mA programmed by I2C per 100 mA.
> +- richtek,fast-uamp : Current of fast-charge mode. The fast-charge current
> +  levels are 700 mA to 2000 mA programmed by I2C per 100 mA.
> +- richtek,eoc-uamp : This property is end of charge current. Its level 150 mA
> +  to 200 mA.
> +- richtek,pre-threshold-uvolt : Voltage of threshold pre-charge mode. Battery
> +  voltage is below pre-charge threshold voltage, the charger is in pre-charge
> +  mode with pre-charge current. Its levels are 2.3 V  to 3.8 V programmed
> +  by I2C per 0.1 V.
> +- richtek,const-uvolt :  Battery regulation voltage of constant voltage mode.
> +  This voltage level 3.65 V to 4.4 V bye I2C per 0.025 V.

These require a Regulator (or Charger?) Ack.

> +	charger {
> +		compatible = "richtek,rt5033-charger";
> +		richtek,pre-uamp = <350000>;
> +		richtek,fast-uamp = <2000000>;
> +		richtek,eoc-uamp = <250000>;
> +		richtek,pre-threshold-uvolt = <3400000>;
> +		richtek,const-uvolt = <4350000>;
> +
> +	};
> +
> +
> +Fuelgauge: There fuelgauge of RT5033 to be instantiated node named "fuelgauge"
> +using the following format.
> +
> +Required properties:
> +- compatible = Must be "richtek,rt5033-battery".
> +
> +	rt5033@35 {
> +		compatible = "richtek,rt5033-battery";
> +		interrupt-parent = <&gpx2>;
> +		interrupts = <3 0>;
> +		reg = <0x35>;
> +	};
> +
> +Example:
> +
> +		rt5033@34 {
> +			compatible = "richtek,rt5033";
> +			reg = <0x34>;
> +			interrupt-parent = <&gpx1>;
> +			interrupts = <5 0>;
> +
> +			regulators {
> +				buck_reg: BUCK {
> +					regulator-name = "BUCK";
> +					regulator-min-microvolt = <1200000>;
> +					regulator-max-microvolt = <1200000>;
> +					regulator-always-on;
> +				};
> +			};
> +
> +			charger {
> +				compatible = "richtek,rt5033-charger";
> +				richtek,pre-uamp = <350000>;
> +				richtek,fast-uamp = <2000000>;
> +				richtek,eoc-uamp = <250000>;
> +				richtek,pre-threshold-uvolt = <3400000>;
> +				richtek,const-uvolt = <4350000>;
> +			};
> +
> +		};
> +
> +		rt5033@35 {
> +				compatible = "richtek,rt5033-battery";
> +				interrupt-parent = <&gpx2>;
> +				interrupts = <3 0>;
> +				reg = <0x35>;
> +		};
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index 389ca13..73d235a 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -149,6 +149,7 @@ ralink	Mediatek/Ralink Technology Corp.
>  ramtron	Ramtron International
>  realtek Realtek Semiconductor Corp.
>  renesas	Renesas Electronics Corporation
> +richtek	Richtek Technology Corporation
>  ricoh	Ricoh Co. Ltd.
>  rockchip	Fuzhou Rockchip Electronics Co., Ltd
>  samsung	Samsung Semiconductor

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v5 1/2] power: rt5033_charger: Add RT5033 charger device driver

[Sebastian Reichel]

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

Hi,

On Mon, Mar 09, 2015 at 01:53:58PM +0900, Beomho Seo wrote:
> >> remove the special handling of POWER_SUPPLY_PROP_CURRENT_MAX in 
> >> rt5033_get_charger_current() and do it like this:
> >>
> >> case POWER_SUPPLY_PROP_CURRENT_NOW:
> >>     val->intval = rt5033_get_charger_current(charger);
> >>     break;
> >> case POWER_SUPPLY_PROP_CURRENT_MAX:
> >>     val->intval = RT5033_CHG_MAX_CURRENT;
> >>     break;
> >>
> > 
> > OK. I will change comply with your comment.
> > 
> 
> RT5033_CHG_MAX_CURRENT is register value(hex).
> So It is better:
> 
> case POWER_SUPPLY_PROP_CURRENT_MAX:
> 	val->intval = RT5033_CHARGER_FAST_CURRENT_MAX;
> 
> And then I will fix rt5033_get_charger_current function more readable.

Right, I copied the wrong value.

> [...]

-- Sebastian

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

Re: [PATCH v5 1/2] power: rt5033_charger: Add RT5033 charger device driver

[Paul Bolle]

On Mon, 2015-03-09 at 10:23 +0900, Beomho Seo wrote:
> --- /dev/null
> +++ b/drivers/power/rt5033_charger.c
> @@ -0,0 +1,485 @@
> +/*
> + * Battery charger driver for RT5033
> + *
> + * Copyright (C) 2014 Samsung Electronics, Co., Ltd.
> + * Author: Beomho Seo <beomho.seo@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published bythe Free Software Foundation.
> + */

(bythe?)

This states the license is GPL v2.

> +MODULE_LICENSE("GPL");

So you probably meant
    MODULE_LICENSE("GPL v2");


Paul Bolle

Re: [PATCH v5 1/2] power: rt5033_charger: Add RT5033 charger device driver

[Beomho Seo]

On 03/11/2015 08:06 PM, Paul Bolle wrote:
> On Mon, 2015-03-09 at 10:23 +0900, Beomho Seo wrote:
>> --- /dev/null
>> +++ b/drivers/power/rt5033_charger.c
>> @@ -0,0 +1,485 @@
>> +/*
>> + * Battery charger driver for RT5033
>> + *
>> + * Copyright (C) 2014 Samsung Electronics, Co., Ltd.
>> + * Author: Beomho Seo <beomho.seo@samsung.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published bythe Free Software Foundation.
>> + */
> 
> (bythe?)
> 
> This states the license is GPL v2.
> 
>> +MODULE_LICENSE("GPL");
> 
> So you probably meant
>     MODULE_LICENSE("GPL v2");
> 
> 
> Paul Bolle
> 

OK I will fix wrong spacing and state the license.
Additionally, fix wrong spacing all rt5033-* driver.

Best regards,
Beomho Seo