2020-12-28 11:37:14

by Timon Baetz

[permalink] [raw]
Subject: [PATCH v5 1/8] extcon: max8997: Add CHGINS and CHGRM interrupt handling

This allows the MAX8997 charger to set the current limit depending on
the detected extcon charger type.

Signed-off-by: Timon Baetz <[email protected]>
---
drivers/extcon/extcon-max8997.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/extcon/extcon-max8997.c b/drivers/extcon/extcon-max8997.c
index 337b0eea4e62..e1408075ef7d 100644
--- a/drivers/extcon/extcon-max8997.c
+++ b/drivers/extcon/extcon-max8997.c
@@ -44,6 +44,8 @@ static struct max8997_muic_irq muic_irqs[] = {
{ MAX8997_MUICIRQ_ChgDetRun, "muic-CHGDETRUN" },
{ MAX8997_MUICIRQ_ChgTyp, "muic-CHGTYP" },
{ MAX8997_MUICIRQ_OVP, "muic-OVP" },
+ { MAX8997_PMICIRQ_CHGINS, "pmic-CHGINS" },
+ { MAX8997_PMICIRQ_CHGRM, "pmic-CHGRM" },
};

/* Define supported cable type */
@@ -538,6 +540,8 @@ static void max8997_muic_irq_work(struct work_struct *work)
case MAX8997_MUICIRQ_DCDTmr:
case MAX8997_MUICIRQ_ChgDetRun:
case MAX8997_MUICIRQ_ChgTyp:
+ case MAX8997_PMICIRQ_CHGINS:
+ case MAX8997_PMICIRQ_CHGRM:
/* Handle charger cable */
ret = max8997_muic_chg_handler(info);
break;
--
2.25.1



2020-12-28 11:37:35

by Timon Baetz

[permalink] [raw]
Subject: [PATCH v5 3/8] power: supply: max8997_charger: Set CHARGER current limit

Register for extcon notification and set charging current depending on
the detected cable type. Current values are taken from vendor kernel,
where most charger types end up setting 650mA [0].

Also enable and disable the CHARGER regulator based on extcon events.

[0] https://github.com/krzk/linux-vendor-backup/blob/samsung/galaxy-s2-epic-4g-touch-sph-d710-exynos4210-dump/drivers/misc/max8997-muic.c#L1675-L1678

Signed-off-by: Timon Baetz <[email protected]>
---
drivers/power/supply/max8997_charger.c | 96 ++++++++++++++++++++++++++
1 file changed, 96 insertions(+)

diff --git a/drivers/power/supply/max8997_charger.c b/drivers/power/supply/max8997_charger.c
index 1947af25879a..f0f725385dfc 100644
--- a/drivers/power/supply/max8997_charger.c
+++ b/drivers/power/supply/max8997_charger.c
@@ -6,12 +6,14 @@
// MyungJoo Ham <[email protected]>

#include <linux/err.h>
+#include <linux/extcon.h>
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/platform_device.h>
#include <linux/power_supply.h>
#include <linux/mfd/max8997.h>
#include <linux/mfd/max8997-private.h>
+#include <linux/regulator/consumer.h>

/* MAX8997_REG_STATUS4 */
#define DCINOK_SHIFT 1
@@ -31,6 +33,10 @@ struct charger_data {
struct device *dev;
struct max8997_dev *iodev;
struct power_supply *battery;
+ struct regulator *reg;
+ struct extcon_dev *edev;
+ struct notifier_block extcon_nb;
+ struct work_struct extcon_work;
};

static enum power_supply_property max8997_battery_props[] = {
@@ -88,6 +94,67 @@ static int max8997_battery_get_property(struct power_supply *psy,
return 0;
}

+static void max8997_battery_extcon_evt_stop_work(void *data)
+{
+ struct charger_data *charger = data;
+
+ cancel_work_sync(&charger->extcon_work);
+}
+
+static void max8997_battery_extcon_evt_worker(struct work_struct *work)
+{
+ struct charger_data *charger =
+ container_of(work, struct charger_data, extcon_work);
+ struct extcon_dev *edev = charger->edev;
+ int current_limit;
+
+ if (extcon_get_state(edev, EXTCON_CHG_USB_SDP) > 0) {
+ dev_dbg(charger->dev, "USB SDP charger is connected\n");
+ current_limit = 450000;
+ } else if (extcon_get_state(edev, EXTCON_CHG_USB_DCP) > 0) {
+ dev_dbg(charger->dev, "USB DCP charger is connected\n");
+ current_limit = 650000;
+ } else if (extcon_get_state(edev, EXTCON_CHG_USB_FAST) > 0) {
+ dev_dbg(charger->dev, "USB FAST charger is connected\n");
+ current_limit = 650000;
+ } else if (extcon_get_state(edev, EXTCON_CHG_USB_SLOW) > 0) {
+ dev_dbg(charger->dev, "USB SLOW charger is connected\n");
+ current_limit = 650000;
+ } else if (extcon_get_state(edev, EXTCON_CHG_USB_CDP) > 0) {
+ dev_dbg(charger->dev, "USB CDP charger is connected\n");
+ current_limit = 650000;
+ } else {
+ dev_dbg(charger->dev, "USB charger is diconnected\n");
+ current_limit = -1;
+ }
+
+ if (current_limit > 0) {
+ int ret = regulator_set_current_limit(charger->reg, current_limit, current_limit);
+
+ if (ret) {
+ dev_err(charger->dev, "failed to set current limit: %d\n", ret);
+ return;
+ }
+ ret = regulator_enable(charger->reg);
+ if (ret)
+ dev_err(charger->dev, "failed to enable regulator: %d\n", ret);
+ } else {
+ int ret = regulator_disable(charger->reg);
+
+ if (ret)
+ dev_err(charger->dev, "failed to disable regulator: %d\n", ret);
+ }
+}
+
+static int max8997_battery_extcon_evt(struct notifier_block *nb,
+ unsigned long event, void *param)
+{
+ struct charger_data *charger =
+ container_of(nb, struct charger_data, extcon_nb);
+ schedule_work(&charger->extcon_work);
+ return NOTIFY_OK;
+}
+
static const struct power_supply_desc max8997_battery_desc = {
.name = "max8997_pmic",
.type = POWER_SUPPLY_TYPE_BATTERY,
@@ -170,6 +237,35 @@ static int max8997_battery_probe(struct platform_device *pdev)
return PTR_ERR(charger->battery);
}

+ charger->reg = devm_regulator_get_optional(&pdev->dev, "charger");
+ if (IS_ERR(charger->reg)) {
+ if (PTR_ERR(charger->reg) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ dev_err(&pdev->dev, "couldn't get charger regulator\n");
+ }
+ charger->edev = extcon_get_edev_by_phandle(&pdev->dev, 0);
+ if (IS_ERR(charger->edev)) {
+ if (PTR_ERR(charger->edev) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ dev_err(charger->dev, "couldn't get extcon device\n");
+ }
+
+ if (!IS_ERR(charger->reg) && !IS_ERR(charger->edev)) {
+ INIT_WORK(&charger->extcon_work, max8997_battery_extcon_evt_worker);
+ ret = devm_add_action(&pdev->dev, max8997_battery_extcon_evt_stop_work, charger);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to add extcon evt stop action: %d\n", ret);
+ return ret;
+ }
+ charger->extcon_nb.notifier_call = max8997_battery_extcon_evt;
+ ret = devm_extcon_register_notifier_all(&pdev->dev, charger->edev,
+ &charger->extcon_nb);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register extcon notifier\n");
+ return ret;
+ };
+ }
+
return 0;
}

--
2.25.1


2020-12-28 11:38:28

by Timon Baetz

[permalink] [raw]
Subject: [PATCH v5 2/8] regulator: dt-bindings: Document max8997-pmic nodes

Add maxim,max8997-battery and maxim,max8997-muic optional nodes.

Signed-off-by: Timon Baetz <[email protected]>
Acked-by: Krzysztof Kozlowski <[email protected]>
---
.../bindings/regulator/max8997-regulator.txt | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/max8997-regulator.txt b/Documentation/devicetree/bindings/regulator/max8997-regulator.txt
index 6fe825b8ac1b..faaf2bbf0272 100644
--- a/Documentation/devicetree/bindings/regulator/max8997-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/max8997-regulator.txt
@@ -53,6 +53,18 @@ Additional properties required if either of the optional properties are used:
- max8997,pmic-buck125-dvs-gpios: GPIO specifiers for three host gpio's used
for dvs. The format of the gpio specifier depends in the gpio controller.

+Optional nodes:
+- charger: Node for configuring the charger driver.
+ Required properties:
+ - compatible: "maxim,max8997-battery"
+ Optional properties:
+ - extcon: extcon specifier for charging events
+ - charger-supply: regulator node for charging current
+
+- muic: Node used only by extcon consumers.
+ Required properties:
+ - compatible: "maxim,max8997-muic"
+
Regulators: The regulators of max8997 that have to be instantiated should be
included in a sub-node named 'regulators'. Regulator nodes included in this
sub-node should be of the format as listed below.
--
2.25.1


2020-12-28 11:38:43

by Timon Baetz

[permalink] [raw]
Subject: [PATCH v5 4/8] ARM: dts: exynos: Add muic and charger nodes for I9100

muic node is only used for extcon consumers.
charger node is used to point to muic and regulator.

Signed-off-by: Timon Baetz <[email protected]>
---
arch/arm/boot/dts/exynos4210-i9100.dts | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts b/arch/arm/boot/dts/exynos4210-i9100.dts
index 5370ee477186..8fa704babd5e 100644
--- a/arch/arm/boot/dts/exynos4210-i9100.dts
+++ b/arch/arm/boot/dts/exynos4210-i9100.dts
@@ -584,6 +584,16 @@ EN32KHZ_CP {
regulator-always-on;
};
};
+
+ muic: max8997-muic {
+ compatible = "maxim,max8997-muic";
+ };
+
+ charger {
+ compatible = "maxim,max8997-battery";
+ charger-supply = <&charger_reg>;
+ extcon = <&muic>;
+ };
};
};

--
2.25.1


2020-12-28 11:39:33

by Timon Baetz

[permalink] [raw]
Subject: [PATCH v5 7/8] ARM: dts: exynos: Fix charging regulator voltage and current for I9100

Set CHARGER current and CHARGER_CV voltage according to Galaxy S2 Epic
4G Touch SPH-D710 Android vendor sources [0,1].

Remove regulator-always-on. The regulator can be enabled and disabled
based on extcon events.

[0] https://github.com/krzk/linux-vendor-backup/blob/samsung/galaxy-s2-epic-4g-touch-sph-d710-exynos4210-dump/drivers/power/max8997_charger_u1.c#L169-L170
[1] https://github.com/krzk/linux-vendor-backup/blob/samsung/galaxy-s2-epic-4g-touch-sph-d710-exynos4210-dump/drivers/power/max8997_charger_u1.c#L390-L391

Signed-off-by: Timon Baetz <[email protected]>
---
arch/arm/boot/dts/exynos4210-i9100.dts | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4210-i9100.dts b/arch/arm/boot/dts/exynos4210-i9100.dts
index 8fa704babd5e..586d801af0b5 100644
--- a/arch/arm/boot/dts/exynos4210-i9100.dts
+++ b/arch/arm/boot/dts/exynos4210-i9100.dts
@@ -562,15 +562,14 @@ safe2_sreg: ESAFEOUT2 {

charger_reg: CHARGER {
regulator-name = "CHARGER";
- regulator-min-microamp = <60000>;
- regulator-max-microamp = <2580000>;
- regulator-always-on;
+ regulator-min-microamp = <200000>;
+ regulator-max-microamp = <950000>;
};

chargercv_reg: CHARGER_CV {
regulator-name = "CHARGER_CV";
- regulator-min-microvolt = <3800000>;
- regulator-max-microvolt = <4100000>;
+ regulator-min-microvolt = <4200000>;
+ regulator-max-microvolt = <4200000>;
regulator-always-on;
};

--
2.25.1


2020-12-30 08:55:04

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v5 3/8] power: supply: max8997_charger: Set CHARGER current limit

On Mon, Dec 28, 2020 at 11:36:02AM +0000, Timon Baetz wrote:
> Register for extcon notification and set charging current depending on
> the detected cable type. Current values are taken from vendor kernel,
> where most charger types end up setting 650mA [0].
>
> Also enable and disable the CHARGER regulator based on extcon events.
>
> [0] https://github.com/krzk/linux-vendor-backup/blob/samsung/galaxy-s2-epic-4g-touch-sph-d710-exynos4210-dump/drivers/misc/max8997-muic.c#L1675-L1678
>
> Signed-off-by: Timon Baetz <[email protected]>
> ---
> drivers/power/supply/max8997_charger.c | 96 ++++++++++++++++++++++++++
> 1 file changed, 96 insertions(+)
>
> diff --git a/drivers/power/supply/max8997_charger.c b/drivers/power/supply/max8997_charger.c
> index 1947af25879a..f0f725385dfc 100644
> --- a/drivers/power/supply/max8997_charger.c
> +++ b/drivers/power/supply/max8997_charger.c
> @@ -6,12 +6,14 @@
> // MyungJoo Ham <[email protected]>
>
> #include <linux/err.h>
> +#include <linux/extcon.h>
> #include <linux/module.h>
> #include <linux/slab.h>
> #include <linux/platform_device.h>
> #include <linux/power_supply.h>
> #include <linux/mfd/max8997.h>
> #include <linux/mfd/max8997-private.h>
> +#include <linux/regulator/consumer.h>
>
> /* MAX8997_REG_STATUS4 */
> #define DCINOK_SHIFT 1
> @@ -31,6 +33,10 @@ struct charger_data {
> struct device *dev;
> struct max8997_dev *iodev;
> struct power_supply *battery;
> + struct regulator *reg;
> + struct extcon_dev *edev;
> + struct notifier_block extcon_nb;
> + struct work_struct extcon_work;
> };
>
> static enum power_supply_property max8997_battery_props[] = {
> @@ -88,6 +94,67 @@ static int max8997_battery_get_property(struct power_supply *psy,
> return 0;
> }
>
> +static void max8997_battery_extcon_evt_stop_work(void *data)
> +{
> + struct charger_data *charger = data;
> +
> + cancel_work_sync(&charger->extcon_work);
> +}
> +
> +static void max8997_battery_extcon_evt_worker(struct work_struct *work)
> +{
> + struct charger_data *charger =
> + container_of(work, struct charger_data, extcon_work);
> + struct extcon_dev *edev = charger->edev;
> + int current_limit;
> +
> + if (extcon_get_state(edev, EXTCON_CHG_USB_SDP) > 0) {
> + dev_dbg(charger->dev, "USB SDP charger is connected\n");
> + current_limit = 450000;
> + } else if (extcon_get_state(edev, EXTCON_CHG_USB_DCP) > 0) {
> + dev_dbg(charger->dev, "USB DCP charger is connected\n");
> + current_limit = 650000;
> + } else if (extcon_get_state(edev, EXTCON_CHG_USB_FAST) > 0) {
> + dev_dbg(charger->dev, "USB FAST charger is connected\n");
> + current_limit = 650000;
> + } else if (extcon_get_state(edev, EXTCON_CHG_USB_SLOW) > 0) {
> + dev_dbg(charger->dev, "USB SLOW charger is connected\n");
> + current_limit = 650000;
> + } else if (extcon_get_state(edev, EXTCON_CHG_USB_CDP) > 0) {
> + dev_dbg(charger->dev, "USB CDP charger is connected\n");
> + current_limit = 650000;
> + } else {
> + dev_dbg(charger->dev, "USB charger is diconnected\n");
> + current_limit = -1;
> + }
> +
> + if (current_limit > 0) {
> + int ret = regulator_set_current_limit(charger->reg, current_limit, current_limit);
> +
> + if (ret) {
> + dev_err(charger->dev, "failed to set current limit: %d\n", ret);
> + return;
> + }
> + ret = regulator_enable(charger->reg);
> + if (ret)
> + dev_err(charger->dev, "failed to enable regulator: %d\n", ret);
> + } else {
> + int ret = regulator_disable(charger->reg);
> +
> + if (ret)
> + dev_err(charger->dev, "failed to disable regulator: %d\n", ret);
> + }
> +}
> +
> +static int max8997_battery_extcon_evt(struct notifier_block *nb,
> + unsigned long event, void *param)
> +{
> + struct charger_data *charger =
> + container_of(nb, struct charger_data, extcon_nb);
> + schedule_work(&charger->extcon_work);
> + return NOTIFY_OK;
> +}
> +
> static const struct power_supply_desc max8997_battery_desc = {
> .name = "max8997_pmic",
> .type = POWER_SUPPLY_TYPE_BATTERY,
> @@ -170,6 +237,35 @@ static int max8997_battery_probe(struct platform_device *pdev)
> return PTR_ERR(charger->battery);
> }
>
> + charger->reg = devm_regulator_get_optional(&pdev->dev, "charger");
> + if (IS_ERR(charger->reg)) {
> + if (PTR_ERR(charger->reg) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> + dev_err(&pdev->dev, "couldn't get charger regulator\n");

This should be dev_info, as we discussed. Otherwise it will scream on
every boot with a DTS which does not have charger node.

Best regards,
Krzysztof

2020-12-30 08:59:53

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v5 1/8] extcon: max8997: Add CHGINS and CHGRM interrupt handling

On Mon, Dec 28, 2020 at 11:35:38AM +0000, Timon Baetz wrote:
> This allows the MAX8997 charger to set the current limit depending on
> the detected extcon charger type.
>
> Signed-off-by: Timon Baetz <[email protected]>
> ---
> drivers/extcon/extcon-max8997.c | 4 ++++
> 1 file changed, 4 insertions(+)

It's already a v5 but what are the changes here? You must provide the
changelog for your patches - either in the cover letter or in each patch.

Best regards,
Krzysztof