* [PATCH 0/5] Remaining patches for RT5033 charger
[not found] <cover.1695844349.git.jahau.ref@rocketmail.com>
@ 2023-09-27 20:25 ` Jakob Hauser
2023-09-27 20:25 ` [PATCH 1/5] fixup for "power: Explicitly include correct DT includes" Jakob Hauser
` (5 more replies)
0 siblings, 6 replies; 11+ messages in thread
From: Jakob Hauser @ 2023-09-27 20:25 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Lee Jones, Stephen Rothwell, Yang Yingliang, Christophe Jaillet,
Stephan Gerhold, Raymond Hackley, Henrik Grimler, linux-pm,
linux-kernel, ~postmarketos/upstreaming, Jakob Hauser
This series collects remaining patches for the RT5033 charger driver.
The patches are based on 6.6-rc1.
Mailing list links where the patches were taken from:
- Patch 1
https://lore.kernel.org/linux-next/20230821125741.3a2474d7@canb.auug.org.au
- Patch 2
https://lore.kernel.org/linux-pm/cover.1684182964.git.jahau@rocketmail.com/T/#m27164a05bbe2ec649be1fc32157b34ba814ff31f
- Patch 3
https://lore.kernel.org/linux-pm/20230822030207.644738-1-yangyingliang@huawei.com/T/#u
- Patches 4 & 5
https://lore.kernel.org/linux-pm/cover.1686948074.git.jahau@rocketmail.com/T/#u
Jakob Hauser (3):
power: supply: rt5033_charger: Add cable detection and USB OTG supply
power: supply: rt5033_charger: Simplify initialization of
rt5033_charger_data
power: supply: rt5033_charger: Replace "&pdev->dev" by "charger->dev"
in probe
Stephen Rothwell (1):
fixup for "power: Explicitly include correct DT includes"
Yang Yingliang (1):
power: supply: rt5033_charger: fix missing unlock
drivers/power/supply/rt5033_charger.c | 320 ++++++++++++++++++++++++--
1 file changed, 299 insertions(+), 21 deletions(-)
--
2.39.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/5] fixup for "power: Explicitly include correct DT includes"
2023-09-27 20:25 ` [PATCH 0/5] Remaining patches for RT5033 charger Jakob Hauser
@ 2023-09-27 20:25 ` Jakob Hauser
2023-09-27 20:25 ` [PATCH 2/5] power: supply: rt5033_charger: Add cable detection and USB OTG supply Jakob Hauser
` (4 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Jakob Hauser @ 2023-09-27 20:25 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Lee Jones, Stephen Rothwell, Yang Yingliang, Christophe Jaillet,
Stephan Gerhold, Raymond Hackley, Henrik Grimler, linux-pm,
linux-kernel, ~postmarketos/upstreaming, Jakob Hauser
From: Stephen Rothwell <sfr@canb.auug.org.au>
interacting with commit
12cc585f36b8 ("power: supply: rt5033_charger: Add cable detection and USB OTG supply")
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
---
drivers/power/supply/rt5033_charger.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/power/supply/rt5033_charger.c b/drivers/power/supply/rt5033_charger.c
index c0c516f22c66..57a0dc631e85 100644
--- a/drivers/power/supply/rt5033_charger.c
+++ b/drivers/power/supply/rt5033_charger.c
@@ -8,6 +8,7 @@
#include <linux/mod_devicetable.h>
#include <linux/module.h>
+#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/power_supply.h>
#include <linux/regmap.h>
--
2.39.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/5] power: supply: rt5033_charger: Add cable detection and USB OTG supply
2023-09-27 20:25 ` [PATCH 0/5] Remaining patches for RT5033 charger Jakob Hauser
2023-09-27 20:25 ` [PATCH 1/5] fixup for "power: Explicitly include correct DT includes" Jakob Hauser
@ 2023-09-27 20:25 ` Jakob Hauser
2023-09-28 5:21 ` Marion & Christophe JAILLET
2023-09-27 20:26 ` [PATCH 3/5] power: supply: rt5033_charger: fix missing unlock Jakob Hauser
` (3 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Jakob Hauser @ 2023-09-27 20:25 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Lee Jones, Stephen Rothwell, Yang Yingliang, Christophe Jaillet,
Stephan Gerhold, Raymond Hackley, Henrik Grimler, linux-pm,
linux-kernel, ~postmarketos/upstreaming, Jakob Hauser,
Sebastian Reichel
Implement cable detection by extcon and handle the driver according to the
connector type.
There are basically three types of action: "set_charging", "set_otg" and
"set_disconnect".
A forth helper function to "unset_otg" was added because this is used in both
"set_charging" and "set_disconnect". In the first case it covers the rather
rare event that someone changes from OTG to charging without disconnect. In
the second case, when disconnecting, the values are set back to the ones from
initialization to return into a defined state.
Additionally, there is "set_mivr". When connecting to e.g. a laptop/PC, the
minimum input voltage regulation (MIVR) shall prevent a voltage drop if the
cable or the supply is weak. The MIVR value is set to 4600MV, same as in the
Android driver [1]. When disconnecting, MIVR is set back to DISABLED.
In the function rt5033_get_charger_state(): When in OTG mode, the chip
reports status "charging". Change this to "discharging" because there is
no charging going on in OTG mode [2].
[1] https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/drivers/battery/rt5033_charger.c#L499
[2] https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/drivers/battery/rt5033_charger.c#L686-L687
Tested-by: Raymond Hackley <raymondhackley@protonmail.com>
Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/power/supply/rt5033_charger.c | 276 +++++++++++++++++++++++++-
1 file changed, 274 insertions(+), 2 deletions(-)
diff --git a/drivers/power/supply/rt5033_charger.c b/drivers/power/supply/rt5033_charger.c
index 57a0dc631e85..2c2073b8979d 100644
--- a/drivers/power/supply/rt5033_charger.c
+++ b/drivers/power/supply/rt5033_charger.c
@@ -6,8 +6,11 @@
* Author: Beomho Seo <beomho.seo@samsung.com>
*/
+#include <linux/devm-helpers.h>
+#include <linux/extcon.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
+#include <linux/mutex.h>
#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/power_supply.h>
@@ -27,6 +30,14 @@ struct rt5033_charger {
struct regmap *regmap;
struct power_supply *psy;
struct rt5033_charger_data *chg;
+ struct extcon_dev *edev;
+ struct notifier_block extcon_nb;
+ struct work_struct extcon_work;
+ struct mutex lock;
+ bool online;
+ bool otg;
+ bool mivr_enabled;
+ u8 cv_regval;
};
static int rt5033_get_charger_state(struct rt5033_charger *charger)
@@ -57,6 +68,10 @@ static int rt5033_get_charger_state(struct rt5033_charger *charger)
state = POWER_SUPPLY_STATUS_UNKNOWN;
}
+ /* For OTG mode, RT5033 would still report "charging" */
+ if (charger->otg)
+ state = POWER_SUPPLY_STATUS_DISCHARGING;
+
return state;
}
@@ -148,6 +163,9 @@ static inline int rt5033_init_const_charge(struct rt5033_charger *charger)
return -EINVAL;
}
+ /* Store that value for later usage */
+ charger->cv_regval = reg_data;
+
/* Set end of charge current */
if (chg->eoc_uamp < RT5033_CHARGER_EOC_MIN ||
chg->eoc_uamp > RT5033_CHARGER_EOC_MAX) {
@@ -331,6 +349,152 @@ static int rt5033_charger_reg_init(struct rt5033_charger *charger)
return 0;
}
+static int rt5033_charger_set_otg(struct rt5033_charger *charger)
+{
+ int ret;
+
+ mutex_lock(&charger->lock);
+
+ /* Set OTG boost v_out to 5 volts */
+ ret = regmap_update_bits(charger->regmap, RT5033_REG_CHG_CTRL2,
+ RT5033_CHGCTRL2_CV_MASK,
+ 0x37 << RT5033_CHGCTRL2_CV_SHIFT);
+ if (ret) {
+ dev_err(charger->dev, "Failed set OTG boost v_out\n");
+ return -EINVAL;
+ }
+
+ /* Set operation mode to OTG */
+ ret = regmap_update_bits(charger->regmap, RT5033_REG_CHG_CTRL1,
+ RT5033_CHGCTRL1_MODE_MASK, RT5033_BOOST_MODE);
+ if (ret) {
+ dev_err(charger->dev, "Failed to update OTG mode.\n");
+ return -EINVAL;
+ }
+
+ /* In case someone switched from charging to OTG directly */
+ if (charger->online)
+ charger->online = false;
+
+ charger->otg = true;
+
+ mutex_unlock(&charger->lock);
+
+ return 0;
+}
+
+static int rt5033_charger_unset_otg(struct rt5033_charger *charger)
+{
+ int ret;
+ u8 data;
+
+ /* Restore constant voltage for charging */
+ data = charger->cv_regval;
+ ret = regmap_update_bits(charger->regmap, RT5033_REG_CHG_CTRL2,
+ RT5033_CHGCTRL2_CV_MASK,
+ data << RT5033_CHGCTRL2_CV_SHIFT);
+ if (ret) {
+ dev_err(charger->dev, "Failed to restore constant voltage\n");
+ return -EINVAL;
+ }
+
+ /* Set operation mode to charging */
+ ret = regmap_update_bits(charger->regmap, RT5033_REG_CHG_CTRL1,
+ RT5033_CHGCTRL1_MODE_MASK, RT5033_CHARGER_MODE);
+ if (ret) {
+ dev_err(charger->dev, "Failed to update charger mode.\n");
+ return -EINVAL;
+ }
+
+ charger->otg = false;
+
+ return 0;
+}
+
+static int rt5033_charger_set_charging(struct rt5033_charger *charger)
+{
+ int ret;
+
+ mutex_lock(&charger->lock);
+
+ /* In case someone switched from OTG to charging directly */
+ if (charger->otg) {
+ ret = rt5033_charger_unset_otg(charger);
+ if (ret)
+ return -EINVAL;
+ }
+
+ charger->online = true;
+
+ mutex_unlock(&charger->lock);
+
+ return 0;
+}
+
+static int rt5033_charger_set_mivr(struct rt5033_charger *charger)
+{
+ int ret;
+
+ mutex_lock(&charger->lock);
+
+ /*
+ * When connected via USB connector type SDP (Standard Downstream Port),
+ * the minimum input voltage regulation (MIVR) should be enabled. It
+ * prevents an input voltage drop due to insufficient current provided
+ * by the adapter or USB input. As a downside, it may reduces the
+ * charging current and thus slows the charging.
+ */
+ ret = regmap_update_bits(charger->regmap, RT5033_REG_CHG_CTRL4,
+ RT5033_CHGCTRL4_MIVR_MASK, RT5033_CHARGER_MIVR_4600MV);
+ if (ret) {
+ dev_err(charger->dev, "Failed to set MIVR level.\n");
+ return -EINVAL;
+ }
+
+ charger->mivr_enabled = true;
+
+ mutex_unlock(&charger->lock);
+
+ /* Beyond this, do the same steps like setting charging */
+ rt5033_charger_set_charging(charger);
+
+ return 0;
+}
+
+static int rt5033_charger_set_disconnect(struct rt5033_charger *charger)
+{
+ int ret;
+
+ mutex_lock(&charger->lock);
+
+ /* Disable MIVR if enabled */
+ if (charger->mivr_enabled) {
+ ret = regmap_update_bits(charger->regmap,
+ RT5033_REG_CHG_CTRL4,
+ RT5033_CHGCTRL4_MIVR_MASK,
+ RT5033_CHARGER_MIVR_DISABLE);
+ if (ret) {
+ dev_err(charger->dev, "Failed to disable MIVR.\n");
+ return -EINVAL;
+ }
+
+ charger->mivr_enabled = false;
+ }
+
+ if (charger->otg) {
+ ret = rt5033_charger_unset_otg(charger);
+ if (ret)
+ return -EINVAL;
+ }
+
+ if (charger->online)
+ charger->online = false;
+
+ mutex_unlock(&charger->lock);
+
+ return 0;
+}
+
static enum power_supply_property rt5033_charger_props[] = {
POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_CHARGE_TYPE,
@@ -367,8 +531,7 @@ static int rt5033_charger_get_property(struct power_supply *psy,
val->strval = RT5033_MANUFACTURER;
break;
case POWER_SUPPLY_PROP_ONLINE:
- val->intval = (rt5033_get_charger_state(charger) ==
- POWER_SUPPLY_STATUS_CHARGING);
+ val->intval = charger->online;
break;
default:
return -EINVAL;
@@ -403,6 +566,86 @@ static struct rt5033_charger_data *rt5033_charger_dt_init(
return chg;
}
+static void rt5033_charger_extcon_work(struct work_struct *work)
+{
+ struct rt5033_charger *charger =
+ container_of(work, struct rt5033_charger, extcon_work);
+ struct extcon_dev *edev = charger->edev;
+ int connector, state;
+ int ret;
+
+ for (connector = EXTCON_USB_HOST; connector <= EXTCON_CHG_USB_PD;
+ connector++) {
+ state = extcon_get_state(edev, connector);
+ if (state == 1)
+ break;
+ }
+
+ /*
+ * Adding a delay between extcon notification and extcon action. This
+ * makes extcon action execution more reliable. Without the delay the
+ * execution sometimes fails, possibly because the chip is busy or not
+ * ready.
+ */
+ msleep(100);
+
+ switch (connector) {
+ case EXTCON_CHG_USB_SDP:
+ ret = rt5033_charger_set_mivr(charger);
+ if (ret) {
+ dev_err(charger->dev, "failed to set USB mode\n");
+ break;
+ }
+ dev_info(charger->dev, "USB mode. connector type: %d\n",
+ connector);
+ break;
+ case EXTCON_CHG_USB_DCP:
+ case EXTCON_CHG_USB_CDP:
+ case EXTCON_CHG_USB_ACA:
+ case EXTCON_CHG_USB_FAST:
+ case EXTCON_CHG_USB_SLOW:
+ case EXTCON_CHG_WPT:
+ case EXTCON_CHG_USB_PD:
+ ret = rt5033_charger_set_charging(charger);
+ if (ret) {
+ dev_err(charger->dev, "failed to set charging\n");
+ break;
+ }
+ dev_info(charger->dev, "charging. connector type: %d\n",
+ connector);
+ break;
+ case EXTCON_USB_HOST:
+ ret = rt5033_charger_set_otg(charger);
+ if (ret) {
+ dev_err(charger->dev, "failed to set OTG\n");
+ break;
+ }
+ dev_info(charger->dev, "OTG enabled\n");
+ break;
+ default:
+ ret = rt5033_charger_set_disconnect(charger);
+ if (ret) {
+ dev_err(charger->dev, "failed to set disconnect\n");
+ break;
+ }
+ dev_info(charger->dev, "disconnected\n");
+ break;
+ }
+
+ power_supply_changed(charger->psy);
+}
+
+static int rt5033_charger_extcon_notifier(struct notifier_block *nb,
+ unsigned long event, void *param)
+{
+ struct rt5033_charger *charger =
+ container_of(nb, struct rt5033_charger, extcon_nb);
+
+ schedule_work(&charger->extcon_work);
+
+ return NOTIFY_OK;
+}
+
static const struct power_supply_desc rt5033_charger_desc = {
.name = "rt5033-charger",
.type = POWER_SUPPLY_TYPE_USB,
@@ -415,6 +658,7 @@ static int rt5033_charger_probe(struct platform_device *pdev)
{
struct rt5033_charger *charger;
struct power_supply_config psy_cfg = {};
+ struct device_node *np_conn, *np_edev;
int ret;
charger = devm_kzalloc(&pdev->dev, sizeof(*charger), GFP_KERNEL);
@@ -424,6 +668,7 @@ static int rt5033_charger_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, charger);
charger->dev = &pdev->dev;
charger->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+ mutex_init(&charger->lock);
psy_cfg.of_node = pdev->dev.of_node;
psy_cfg.drv_data = charger;
@@ -443,6 +688,33 @@ static int rt5033_charger_probe(struct platform_device *pdev)
if (ret)
return ret;
+ /*
+ * Extcon support is not vital for the charger to work. If no extcon
+ * is available, just emit a warning and leave the probe function.
+ */
+ np_conn = of_parse_phandle(pdev->dev.of_node, "richtek,usb-connector", 0);
+ np_edev = of_get_parent(np_conn);
+ charger->edev = extcon_find_edev_by_node(np_edev);
+ if (IS_ERR(charger->edev)) {
+ dev_warn(&pdev->dev, "no extcon device found in device-tree\n");
+ goto out;
+ }
+
+ ret = devm_work_autocancel(&pdev->dev, &charger->extcon_work,
+ rt5033_charger_extcon_work);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to initialize extcon work\n");
+ return ret;
+ }
+
+ charger->extcon_nb.notifier_call = rt5033_charger_extcon_notifier;
+ 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;
+ }
+out:
return 0;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/5] power: supply: rt5033_charger: fix missing unlock
2023-09-27 20:25 ` [PATCH 0/5] Remaining patches for RT5033 charger Jakob Hauser
2023-09-27 20:25 ` [PATCH 1/5] fixup for "power: Explicitly include correct DT includes" Jakob Hauser
2023-09-27 20:25 ` [PATCH 2/5] power: supply: rt5033_charger: Add cable detection and USB OTG supply Jakob Hauser
@ 2023-09-27 20:26 ` Jakob Hauser
2023-09-28 5:23 ` Marion & Christophe JAILLET
2023-09-27 20:26 ` [PATCH 4/5] power: supply: rt5033_charger: Simplify initialization of rt5033_charger_data Jakob Hauser
` (2 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Jakob Hauser @ 2023-09-27 20:26 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Lee Jones, Stephen Rothwell, Yang Yingliang, Christophe Jaillet,
Stephan Gerhold, Raymond Hackley, Henrik Grimler, linux-pm,
linux-kernel, ~postmarketos/upstreaming, Jakob Hauser
From: Yang Yingliang <yangyingliang@huawei.com>
Fix missing mutex_unlock() in some error path.
Fixes: 12cc585f36b8 ("power: supply: rt5033_charger: Add cable detection and USB OTG supply")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
---
drivers/power/supply/rt5033_charger.c | 28 ++++++++++++++++++---------
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/drivers/power/supply/rt5033_charger.c b/drivers/power/supply/rt5033_charger.c
index 2c2073b8979d..091ca4a21f29 100644
--- a/drivers/power/supply/rt5033_charger.c
+++ b/drivers/power/supply/rt5033_charger.c
@@ -361,7 +361,8 @@ static int rt5033_charger_set_otg(struct rt5033_charger *charger)
0x37 << RT5033_CHGCTRL2_CV_SHIFT);
if (ret) {
dev_err(charger->dev, "Failed set OTG boost v_out\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto out_unlock;
}
/* Set operation mode to OTG */
@@ -369,7 +370,8 @@ static int rt5033_charger_set_otg(struct rt5033_charger *charger)
RT5033_CHGCTRL1_MODE_MASK, RT5033_BOOST_MODE);
if (ret) {
dev_err(charger->dev, "Failed to update OTG mode.\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto out_unlock;
}
/* In case someone switched from charging to OTG directly */
@@ -378,9 +380,10 @@ static int rt5033_charger_set_otg(struct rt5033_charger *charger)
charger->otg = true;
+out_unlock:
mutex_unlock(&charger->lock);
- return 0;
+ return ret;
}
static int rt5033_charger_unset_otg(struct rt5033_charger *charger)
@@ -420,8 +423,10 @@ static int rt5033_charger_set_charging(struct rt5033_charger *charger)
/* In case someone switched from OTG to charging directly */
if (charger->otg) {
ret = rt5033_charger_unset_otg(charger);
- if (ret)
+ if (ret) {
+ mutex_unlock(&charger->lock);
return -EINVAL;
+ }
}
charger->online = true;
@@ -448,6 +453,7 @@ static int rt5033_charger_set_mivr(struct rt5033_charger *charger)
RT5033_CHGCTRL4_MIVR_MASK, RT5033_CHARGER_MIVR_4600MV);
if (ret) {
dev_err(charger->dev, "Failed to set MIVR level.\n");
+ mutex_unlock(&charger->lock);
return -EINVAL;
}
@@ -463,7 +469,7 @@ static int rt5033_charger_set_mivr(struct rt5033_charger *charger)
static int rt5033_charger_set_disconnect(struct rt5033_charger *charger)
{
- int ret;
+ int ret = 0;
mutex_lock(&charger->lock);
@@ -475,7 +481,8 @@ static int rt5033_charger_set_disconnect(struct rt5033_charger *charger)
RT5033_CHARGER_MIVR_DISABLE);
if (ret) {
dev_err(charger->dev, "Failed to disable MIVR.\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto out_unlock;
}
charger->mivr_enabled = false;
@@ -483,16 +490,19 @@ static int rt5033_charger_set_disconnect(struct rt5033_charger *charger)
if (charger->otg) {
ret = rt5033_charger_unset_otg(charger);
- if (ret)
- return -EINVAL;
+ if (ret) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
}
if (charger->online)
charger->online = false;
+out_unlock:
mutex_unlock(&charger->lock);
- return 0;
+ return ret;
}
static enum power_supply_property rt5033_charger_props[] = {
--
2.39.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/5] power: supply: rt5033_charger: Simplify initialization of rt5033_charger_data
2023-09-27 20:25 ` [PATCH 0/5] Remaining patches for RT5033 charger Jakob Hauser
` (2 preceding siblings ...)
2023-09-27 20:26 ` [PATCH 3/5] power: supply: rt5033_charger: fix missing unlock Jakob Hauser
@ 2023-09-27 20:26 ` Jakob Hauser
2023-09-27 20:26 ` [PATCH 5/5] power: supply: rt5033_charger: Replace "&pdev->dev" by "charger->dev" in probe Jakob Hauser
2023-10-04 22:15 ` (subset) [PATCH 0/5] Remaining patches for RT5033 charger Sebastian Reichel
5 siblings, 0 replies; 11+ messages in thread
From: Jakob Hauser @ 2023-09-27 20:26 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Lee Jones, Stephen Rothwell, Yang Yingliang, Christophe Jaillet,
Stephan Gerhold, Raymond Hackley, Henrik Grimler, linux-pm,
linux-kernel, ~postmarketos/upstreaming, Jakob Hauser
Currently the struct "rt5033_charger_data" is initialized rather complicated.
The cause lies inside of the struct "rt5033_charger", where struct
"rt5033_charger_data" is implemented as a pointer *chg.
Therefore, inside of struct "rt5033_charger" change the struct
"rt5033_charger_data" to non-pointer "chg". It is then initialized right
away and can be accessed more easily.
Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
---
drivers/power/supply/rt5033_charger.c | 29 +++++++++++----------------
1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/drivers/power/supply/rt5033_charger.c b/drivers/power/supply/rt5033_charger.c
index 091ca4a21f29..b34ef0ea6f8a 100644
--- a/drivers/power/supply/rt5033_charger.c
+++ b/drivers/power/supply/rt5033_charger.c
@@ -29,7 +29,7 @@ struct rt5033_charger {
struct device *dev;
struct regmap *regmap;
struct power_supply *psy;
- struct rt5033_charger_data *chg;
+ struct rt5033_charger_data chg;
struct extcon_dev *edev;
struct notifier_block extcon_nb;
struct work_struct extcon_work;
@@ -131,7 +131,7 @@ static int rt5033_get_charger_const_voltage(struct rt5033_charger *charger)
static inline int rt5033_init_const_charge(struct rt5033_charger *charger)
{
- struct rt5033_charger_data *chg = charger->chg;
+ struct rt5033_charger_data *chg = &charger->chg;
int ret;
unsigned int val;
u8 reg_data;
@@ -205,7 +205,7 @@ static inline int rt5033_init_const_charge(struct rt5033_charger *charger)
static inline int rt5033_init_fast_charge(struct rt5033_charger *charger)
{
- struct rt5033_charger_data *chg = charger->chg;
+ struct rt5033_charger_data *chg = &charger->chg;
int ret;
unsigned int val;
u8 reg_data;
@@ -250,7 +250,7 @@ static inline int rt5033_init_fast_charge(struct rt5033_charger *charger)
static inline int rt5033_init_pre_charge(struct rt5033_charger *charger)
{
- struct rt5033_charger_data *chg = charger->chg;
+ struct rt5033_charger_data *chg = &charger->chg;
int ret;
unsigned int val;
u8 reg_data;
@@ -550,21 +550,16 @@ static int rt5033_charger_get_property(struct power_supply *psy,
return 0;
}
-static struct rt5033_charger_data *rt5033_charger_dt_init(
- struct rt5033_charger *charger)
+static int rt5033_charger_dt_init(struct rt5033_charger *charger)
{
- struct rt5033_charger_data *chg;
+ struct rt5033_charger_data *chg = &charger->chg;
struct power_supply_battery_info *info;
int ret;
- chg = devm_kzalloc(charger->dev, sizeof(*chg), GFP_KERNEL);
- if (!chg)
- return ERR_PTR(-ENOMEM);
-
ret = power_supply_get_battery_info(charger->psy, &info);
if (ret)
- return ERR_PTR(dev_err_probe(charger->dev, -EINVAL,
- "missing battery info\n"));
+ return dev_err_probe(charger->dev, -EINVAL,
+ "missing battery info\n");
/* Assign data. Validity will be checked in the init functions. */
chg->pre_uamp = info->precharge_current_ua;
@@ -573,7 +568,7 @@ static struct rt5033_charger_data *rt5033_charger_dt_init(
chg->pre_uvolt = info->precharge_voltage_max_uv;
chg->const_uvolt = info->constant_charge_voltage_max_uv;
- return chg;
+ return 0;
}
static void rt5033_charger_extcon_work(struct work_struct *work)
@@ -690,9 +685,9 @@ static int rt5033_charger_probe(struct platform_device *pdev)
return dev_err_probe(&pdev->dev, PTR_ERR(charger->psy),
"Failed to register power supply\n");
- charger->chg = rt5033_charger_dt_init(charger);
- if (IS_ERR_OR_NULL(charger->chg))
- return PTR_ERR(charger->chg);
+ ret = rt5033_charger_dt_init(charger);
+ if (ret)
+ return ret;
ret = rt5033_charger_reg_init(charger);
if (ret)
--
2.39.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 5/5] power: supply: rt5033_charger: Replace "&pdev->dev" by "charger->dev" in probe
2023-09-27 20:25 ` [PATCH 0/5] Remaining patches for RT5033 charger Jakob Hauser
` (3 preceding siblings ...)
2023-09-27 20:26 ` [PATCH 4/5] power: supply: rt5033_charger: Simplify initialization of rt5033_charger_data Jakob Hauser
@ 2023-09-27 20:26 ` Jakob Hauser
2023-10-04 22:15 ` (subset) [PATCH 0/5] Remaining patches for RT5033 charger Sebastian Reichel
5 siblings, 0 replies; 11+ messages in thread
From: Jakob Hauser @ 2023-09-27 20:26 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Lee Jones, Stephen Rothwell, Yang Yingliang, Christophe Jaillet,
Stephan Gerhold, Raymond Hackley, Henrik Grimler, linux-pm,
linux-kernel, ~postmarketos/upstreaming, Jakob Hauser
At the beginning of the probe function, "charger->dev" is set equal to
"&pdev->dev". Therefore it's more clear to subsequently use "charger->dev"
instead of "&pdev->dev".
Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
---
drivers/power/supply/rt5033_charger.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/power/supply/rt5033_charger.c b/drivers/power/supply/rt5033_charger.c
index b34ef0ea6f8a..d19c7e80a92a 100644
--- a/drivers/power/supply/rt5033_charger.c
+++ b/drivers/power/supply/rt5033_charger.c
@@ -678,11 +678,11 @@ static int rt5033_charger_probe(struct platform_device *pdev)
psy_cfg.of_node = pdev->dev.of_node;
psy_cfg.drv_data = charger;
- charger->psy = devm_power_supply_register(&pdev->dev,
+ charger->psy = devm_power_supply_register(charger->dev,
&rt5033_charger_desc,
&psy_cfg);
if (IS_ERR(charger->psy))
- return dev_err_probe(&pdev->dev, PTR_ERR(charger->psy),
+ return dev_err_probe(charger->dev, PTR_ERR(charger->psy),
"Failed to register power supply\n");
ret = rt5033_charger_dt_init(charger);
@@ -701,22 +701,22 @@ static int rt5033_charger_probe(struct platform_device *pdev)
np_edev = of_get_parent(np_conn);
charger->edev = extcon_find_edev_by_node(np_edev);
if (IS_ERR(charger->edev)) {
- dev_warn(&pdev->dev, "no extcon device found in device-tree\n");
+ dev_warn(charger->dev, "no extcon device found in device-tree\n");
goto out;
}
- ret = devm_work_autocancel(&pdev->dev, &charger->extcon_work,
+ ret = devm_work_autocancel(charger->dev, &charger->extcon_work,
rt5033_charger_extcon_work);
if (ret) {
- dev_err(&pdev->dev, "failed to initialize extcon work\n");
+ dev_err(charger->dev, "failed to initialize extcon work\n");
return ret;
}
charger->extcon_nb.notifier_call = rt5033_charger_extcon_notifier;
- ret = devm_extcon_register_notifier_all(&pdev->dev, charger->edev,
+ ret = devm_extcon_register_notifier_all(charger->dev, charger->edev,
&charger->extcon_nb);
if (ret) {
- dev_err(&pdev->dev, "failed to register extcon notifier\n");
+ dev_err(charger->dev, "failed to register extcon notifier\n");
return ret;
}
out:
--
2.39.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/5] power: supply: rt5033_charger: Add cable detection and USB OTG supply
2023-09-27 20:25 ` [PATCH 2/5] power: supply: rt5033_charger: Add cable detection and USB OTG supply Jakob Hauser
@ 2023-09-28 5:21 ` Marion & Christophe JAILLET
0 siblings, 0 replies; 11+ messages in thread
From: Marion & Christophe JAILLET @ 2023-09-28 5:21 UTC (permalink / raw)
To: Jakob Hauser, Sebastian Reichel
Cc: Lee Jones, Stephen Rothwell, Yang Yingliang, Stephan Gerhold,
Raymond Hackley, Henrik Grimler, linux-pm, linux-kernel,
~postmarketos/upstreaming, Sebastian Reichel
Le 27/09/2023 à 22:25, Jakob Hauser a écrit :
> Implement cable detection by extcon and handle the driver according to the
> connector type.
>
> There are basically three types of action: "set_charging", "set_otg" and
> "set_disconnect".
>
> A forth helper function to "unset_otg" was added because this is used in both
> "set_charging" and "set_disconnect". In the first case it covers the rather
> rare event that someone changes from OTG to charging without disconnect. In
> the second case, when disconnecting, the values are set back to the ones from
> initialization to return into a defined state.
>
> Additionally, there is "set_mivr". When connecting to e.g. a laptop/PC, the
> minimum input voltage regulation (MIVR) shall prevent a voltage drop if the
> cable or the supply is weak. The MIVR value is set to 4600MV, same as in the
> Android driver [1]. When disconnecting, MIVR is set back to DISABLED.
>
> In the function rt5033_get_charger_state(): When in OTG mode, the chip
> reports status "charging". Change this to "discharging" because there is
> no charging going on in OTG mode [2].
>
> [1] https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/drivers/battery/rt5033_charger.c#L499
> [2] https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/drivers/battery/rt5033_charger.c#L686-L687
>
> Tested-by: Raymond Hackley <raymondhackley@protonmail.com>
> Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
> Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
> drivers/power/supply/rt5033_charger.c | 276 +++++++++++++++++++++++++-
> 1 file changed, 274 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/power/supply/rt5033_charger.c b/drivers/power/supply/rt5033_charger.c
> index 57a0dc631e85..2c2073b8979d 100644
> --- a/drivers/power/supply/rt5033_charger.c
> +++ b/drivers/power/supply/rt5033_charger.c
> @@ -6,8 +6,11 @@
> * Author: Beomho Seo <beomho.seo@samsung.com>
> */
>
> +#include <linux/devm-helpers.h>
> +#include <linux/extcon.h>
> #include <linux/mod_devicetable.h>
> #include <linux/module.h>
> +#include <linux/mutex.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> #include <linux/power_supply.h>
> @@ -27,6 +30,14 @@ struct rt5033_charger {
> struct regmap *regmap;
> struct power_supply *psy;
> struct rt5033_charger_data *chg;
> + struct extcon_dev *edev;
> + struct notifier_block extcon_nb;
> + struct work_struct extcon_work;
> + struct mutex lock;
> + bool online;
> + bool otg;
> + bool mivr_enabled;
> + u8 cv_regval;
> };
>
> static int rt5033_get_charger_state(struct rt5033_charger *charger)
> @@ -57,6 +68,10 @@ static int rt5033_get_charger_state(struct rt5033_charger *charger)
> state = POWER_SUPPLY_STATUS_UNKNOWN;
> }
>
> + /* For OTG mode, RT5033 would still report "charging" */
> + if (charger->otg)
> + state = POWER_SUPPLY_STATUS_DISCHARGING;
> +
> return state;
> }
>
> @@ -148,6 +163,9 @@ static inline int rt5033_init_const_charge(struct rt5033_charger *charger)
> return -EINVAL;
> }
>
> + /* Store that value for later usage */
> + charger->cv_regval = reg_data;
> +
> /* Set end of charge current */
> if (chg->eoc_uamp < RT5033_CHARGER_EOC_MIN ||
> chg->eoc_uamp > RT5033_CHARGER_EOC_MAX) {
> @@ -331,6 +349,152 @@ static int rt5033_charger_reg_init(struct rt5033_charger *charger)
> return 0;
> }
>
> +static int rt5033_charger_set_otg(struct rt5033_charger *charger)
> +{
> + int ret;
> +
> + mutex_lock(&charger->lock);
> +
> + /* Set OTG boost v_out to 5 volts */
> + ret = regmap_update_bits(charger->regmap, RT5033_REG_CHG_CTRL2,
> + RT5033_CHGCTRL2_CV_MASK,
> + 0x37 << RT5033_CHGCTRL2_CV_SHIFT);
> + if (ret) {
> + dev_err(charger->dev, "Failed set OTG boost v_out\n");
unlock
> + return -EINVAL;
> + }
> +
> + /* Set operation mode to OTG */
> + ret = regmap_update_bits(charger->regmap, RT5033_REG_CHG_CTRL1,
> + RT5033_CHGCTRL1_MODE_MASK, RT5033_BOOST_MODE);
> + if (ret) {
> + dev_err(charger->dev, "Failed to update OTG mode.\n");
unlock
> + return -EINVAL;
> + }
> +
> + /* In case someone switched from charging to OTG directly */
> + if (charger->online)
> + charger->online = false;
> +
> + charger->otg = true;
> +
> + mutex_unlock(&charger->lock);
> +
> + return 0;
> +}
> +
> +static int rt5033_charger_unset_otg(struct rt5033_charger *charger)
> +{
> + int ret;
> + u8 data;
> +
> + /* Restore constant voltage for charging */
> + data = charger->cv_regval;
> + ret = regmap_update_bits(charger->regmap, RT5033_REG_CHG_CTRL2,
> + RT5033_CHGCTRL2_CV_MASK,
> + data << RT5033_CHGCTRL2_CV_SHIFT);
> + if (ret) {
> + dev_err(charger->dev, "Failed to restore constant voltage\n");
> + return -EINVAL;
> + }
> +
> + /* Set operation mode to charging */
> + ret = regmap_update_bits(charger->regmap, RT5033_REG_CHG_CTRL1,
> + RT5033_CHGCTRL1_MODE_MASK, RT5033_CHARGER_MODE);
> + if (ret) {
> + dev_err(charger->dev, "Failed to update charger mode.\n");
> + return -EINVAL;
> + }
> +
> + charger->otg = false;
> +
> + return 0;
> +}
> +
> +static int rt5033_charger_set_charging(struct rt5033_charger *charger)
> +{
> + int ret;
> +
> + mutex_lock(&charger->lock);
> +
> + /* In case someone switched from OTG to charging directly */
> + if (charger->otg) {
> + ret = rt5033_charger_unset_otg(charger);
> + if (ret)
unlock
> + return -EINVAL;
> + }
> +
> + charger->online = true;
> +
> + mutex_unlock(&charger->lock);
> +
> + return 0;
> +}
> +
> +static int rt5033_charger_set_mivr(struct rt5033_charger *charger)
> +{
> + int ret;
> +
> + mutex_lock(&charger->lock);
> +
> + /*
> + * When connected via USB connector type SDP (Standard Downstream Port),
> + * the minimum input voltage regulation (MIVR) should be enabled. It
> + * prevents an input voltage drop due to insufficient current provided
> + * by the adapter or USB input. As a downside, it may reduces the
> + * charging current and thus slows the charging.
> + */
> + ret = regmap_update_bits(charger->regmap, RT5033_REG_CHG_CTRL4,
> + RT5033_CHGCTRL4_MIVR_MASK, RT5033_CHARGER_MIVR_4600MV);
> + if (ret) {
> + dev_err(charger->dev, "Failed to set MIVR level.\n");
unlock
> + return -EINVAL;
> + }
> +
> + charger->mivr_enabled = true;
> +
> + mutex_unlock(&charger->lock);
> +
> + /* Beyond this, do the same steps like setting charging */
> + rt5033_charger_set_charging(charger);
> +
> + return 0;
> +}
> +
> +static int rt5033_charger_set_disconnect(struct rt5033_charger *charger)
> +{
> + int ret;
> +
> + mutex_lock(&charger->lock);
> +
> + /* Disable MIVR if enabled */
> + if (charger->mivr_enabled) {
> + ret = regmap_update_bits(charger->regmap,
> + RT5033_REG_CHG_CTRL4,
> + RT5033_CHGCTRL4_MIVR_MASK,
> + RT5033_CHARGER_MIVR_DISABLE);
> + if (ret) {
> + dev_err(charger->dev, "Failed to disable MIVR.\n");
unlock
> + return -EINVAL;
> + }
> +
> + charger->mivr_enabled = false;
> + }
> +
> + if (charger->otg) {
> + ret = rt5033_charger_unset_otg(charger);
> + if (ret)
unlock
> + return -EINVAL;
> + }
> +
> + if (charger->online)
> + charger->online = false;
> +
> + mutex_unlock(&charger->lock);
> +
> + return 0;
> +}
> +
> static enum power_supply_property rt5033_charger_props[] = {
> POWER_SUPPLY_PROP_STATUS,
> POWER_SUPPLY_PROP_CHARGE_TYPE,
> @@ -367,8 +531,7 @@ static int rt5033_charger_get_property(struct power_supply *psy,
> val->strval = RT5033_MANUFACTURER;
> break;
> case POWER_SUPPLY_PROP_ONLINE:
> - val->intval = (rt5033_get_charger_state(charger) ==
> - POWER_SUPPLY_STATUS_CHARGING);
> + val->intval = charger->online;
> break;
> default:
> return -EINVAL;
> @@ -403,6 +566,86 @@ static struct rt5033_charger_data *rt5033_charger_dt_init(
> return chg;
> }
>
> +static void rt5033_charger_extcon_work(struct work_struct *work)
> +{
> + struct rt5033_charger *charger =
> + container_of(work, struct rt5033_charger, extcon_work);
> + struct extcon_dev *edev = charger->edev;
> + int connector, state;
> + int ret;
> +
> + for (connector = EXTCON_USB_HOST; connector <= EXTCON_CHG_USB_PD;
> + connector++) {
> + state = extcon_get_state(edev, connector);
> + if (state == 1)
> + break;
> + }
> +
> + /*
> + * Adding a delay between extcon notification and extcon action. This
> + * makes extcon action execution more reliable. Without the delay the
> + * execution sometimes fails, possibly because the chip is busy or not
> + * ready.
> + */
> + msleep(100);
> +
> + switch (connector) {
> + case EXTCON_CHG_USB_SDP:
> + ret = rt5033_charger_set_mivr(charger);
> + if (ret) {
> + dev_err(charger->dev, "failed to set USB mode\n");
> + break;
> + }
> + dev_info(charger->dev, "USB mode. connector type: %d\n",
> + connector);
> + break;
> + case EXTCON_CHG_USB_DCP:
> + case EXTCON_CHG_USB_CDP:
> + case EXTCON_CHG_USB_ACA:
> + case EXTCON_CHG_USB_FAST:
> + case EXTCON_CHG_USB_SLOW:
> + case EXTCON_CHG_WPT:
> + case EXTCON_CHG_USB_PD:
> + ret = rt5033_charger_set_charging(charger);
> + if (ret) {
> + dev_err(charger->dev, "failed to set charging\n");
> + break;
> + }
> + dev_info(charger->dev, "charging. connector type: %d\n",
> + connector);
> + break;
> + case EXTCON_USB_HOST:
> + ret = rt5033_charger_set_otg(charger);
> + if (ret) {
> + dev_err(charger->dev, "failed to set OTG\n");
> + break;
> + }
> + dev_info(charger->dev, "OTG enabled\n");
> + break;
> + default:
> + ret = rt5033_charger_set_disconnect(charger);
> + if (ret) {
> + dev_err(charger->dev, "failed to set disconnect\n");
> + break;
> + }
> + dev_info(charger->dev, "disconnected\n");
> + break;
> + }
> +
> + power_supply_changed(charger->psy);
> +}
> +
> +static int rt5033_charger_extcon_notifier(struct notifier_block *nb,
> + unsigned long event, void *param)
> +{
> + struct rt5033_charger *charger =
> + container_of(nb, struct rt5033_charger, extcon_nb);
> +
> + schedule_work(&charger->extcon_work);
> +
> + return NOTIFY_OK;
> +}
> +
> static const struct power_supply_desc rt5033_charger_desc = {
> .name = "rt5033-charger",
> .type = POWER_SUPPLY_TYPE_USB,
> @@ -415,6 +658,7 @@ static int rt5033_charger_probe(struct platform_device *pdev)
> {
> struct rt5033_charger *charger;
> struct power_supply_config psy_cfg = {};
> + struct device_node *np_conn, *np_edev;
> int ret;
>
> charger = devm_kzalloc(&pdev->dev, sizeof(*charger), GFP_KERNEL);
> @@ -424,6 +668,7 @@ static int rt5033_charger_probe(struct platform_device *pdev)
> platform_set_drvdata(pdev, charger);
> charger->dev = &pdev->dev;
> charger->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> + mutex_init(&charger->lock);
>
> psy_cfg.of_node = pdev->dev.of_node;
> psy_cfg.drv_data = charger;
> @@ -443,6 +688,33 @@ static int rt5033_charger_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> + /*
> + * Extcon support is not vital for the charger to work. If no extcon
> + * is available, just emit a warning and leave the probe function.
> + */
> + np_conn = of_parse_phandle(pdev->dev.of_node, "richtek,usb-connector", 0);
> + np_edev = of_get_parent(np_conn);
> + charger->edev = extcon_find_edev_by_node(np_edev);
> + if (IS_ERR(charger->edev)) {
> + dev_warn(&pdev->dev, "no extcon device found in device-tree\n");
> + goto out;
> + }
> +
> + ret = devm_work_autocancel(&pdev->dev, &charger->extcon_work,
> + rt5033_charger_extcon_work);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to initialize extcon work\n");
> + return ret;
> + }
> +
> + charger->extcon_nb.notifier_call = rt5033_charger_extcon_notifier;
> + 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;
> + }
> +out:
> return 0;
> }
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/5] power: supply: rt5033_charger: fix missing unlock
2023-09-27 20:26 ` [PATCH 3/5] power: supply: rt5033_charger: fix missing unlock Jakob Hauser
@ 2023-09-28 5:23 ` Marion & Christophe JAILLET
2023-09-28 19:46 ` Jakob Hauser
0 siblings, 1 reply; 11+ messages in thread
From: Marion & Christophe JAILLET @ 2023-09-28 5:23 UTC (permalink / raw)
To: Jakob Hauser, Sebastian Reichel
Cc: Lee Jones, Stephen Rothwell, Yang Yingliang, Stephan Gerhold,
Raymond Hackley, Henrik Grimler, linux-pm, linux-kernel,
~postmarketos/upstreaming
Ok, but why not already in patch #1?
CJ
Le 27/09/2023 à 22:26, Jakob Hauser a écrit :
> From: Yang Yingliang <yangyingliang@huawei.com>
>
> Fix missing mutex_unlock() in some error path.
>
> Fixes: 12cc585f36b8 ("power: supply: rt5033_charger: Add cable detection and USB OTG supply")
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> Signed-off-by: Jakob Hauser <jahau@rocketmail.com>
> ---
> drivers/power/supply/rt5033_charger.c | 28 ++++++++++++++++++---------
> 1 file changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/power/supply/rt5033_charger.c b/drivers/power/supply/rt5033_charger.c
> index 2c2073b8979d..091ca4a21f29 100644
> --- a/drivers/power/supply/rt5033_charger.c
> +++ b/drivers/power/supply/rt5033_charger.c
> @@ -361,7 +361,8 @@ static int rt5033_charger_set_otg(struct rt5033_charger *charger)
> 0x37 << RT5033_CHGCTRL2_CV_SHIFT);
> if (ret) {
> dev_err(charger->dev, "Failed set OTG boost v_out\n");
> - return -EINVAL;
> + ret = -EINVAL;
> + goto out_unlock;
> }
>
> /* Set operation mode to OTG */
> @@ -369,7 +370,8 @@ static int rt5033_charger_set_otg(struct rt5033_charger *charger)
> RT5033_CHGCTRL1_MODE_MASK, RT5033_BOOST_MODE);
> if (ret) {
> dev_err(charger->dev, "Failed to update OTG mode.\n");
> - return -EINVAL;
> + ret = -EINVAL;
> + goto out_unlock;
> }
>
> /* In case someone switched from charging to OTG directly */
> @@ -378,9 +380,10 @@ static int rt5033_charger_set_otg(struct rt5033_charger *charger)
>
> charger->otg = true;
>
> +out_unlock:
> mutex_unlock(&charger->lock);
>
> - return 0;
> + return ret;
> }
>
> static int rt5033_charger_unset_otg(struct rt5033_charger *charger)
> @@ -420,8 +423,10 @@ static int rt5033_charger_set_charging(struct rt5033_charger *charger)
> /* In case someone switched from OTG to charging directly */
> if (charger->otg) {
> ret = rt5033_charger_unset_otg(charger);
> - if (ret)
> + if (ret) {
> + mutex_unlock(&charger->lock);
> return -EINVAL;
> + }
> }
>
> charger->online = true;
> @@ -448,6 +453,7 @@ static int rt5033_charger_set_mivr(struct rt5033_charger *charger)
> RT5033_CHGCTRL4_MIVR_MASK, RT5033_CHARGER_MIVR_4600MV);
> if (ret) {
> dev_err(charger->dev, "Failed to set MIVR level.\n");
> + mutex_unlock(&charger->lock);
> return -EINVAL;
> }
>
> @@ -463,7 +469,7 @@ static int rt5033_charger_set_mivr(struct rt5033_charger *charger)
>
> static int rt5033_charger_set_disconnect(struct rt5033_charger *charger)
> {
> - int ret;
> + int ret = 0;
>
> mutex_lock(&charger->lock);
>
> @@ -475,7 +481,8 @@ static int rt5033_charger_set_disconnect(struct rt5033_charger *charger)
> RT5033_CHARGER_MIVR_DISABLE);
> if (ret) {
> dev_err(charger->dev, "Failed to disable MIVR.\n");
> - return -EINVAL;
> + ret = -EINVAL;
> + goto out_unlock;
> }
>
> charger->mivr_enabled = false;
> @@ -483,16 +490,19 @@ static int rt5033_charger_set_disconnect(struct rt5033_charger *charger)
>
> if (charger->otg) {
> ret = rt5033_charger_unset_otg(charger);
> - if (ret)
> - return -EINVAL;
> + if (ret) {
> + ret = -EINVAL;
> + goto out_unlock;
> + }
> }
>
> if (charger->online)
> charger->online = false;
>
> +out_unlock:
> mutex_unlock(&charger->lock);
>
> - return 0;
> + return ret;
> }
>
> static enum power_supply_property rt5033_charger_props[] = {
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/5] power: supply: rt5033_charger: fix missing unlock
2023-09-28 5:23 ` Marion & Christophe JAILLET
@ 2023-09-28 19:46 ` Jakob Hauser
2023-09-30 19:45 ` Sebastian Reichel
0 siblings, 1 reply; 11+ messages in thread
From: Jakob Hauser @ 2023-09-28 19:46 UTC (permalink / raw)
To: Marion & Christophe JAILLET
Cc: Sebastian Reichel, Lee Jones, Stephen Rothwell, Yang Yingliang,
Stephan Gerhold, Raymond Hackley, Henrik Grimler, linux-pm,
linux-kernel, ~postmarketos/upstreaming
Hi Christophe,
On 28.09.23 07:23, Marion & Christophe JAILLET wrote:
> Ok, but why not already in patch #1?
Thanks for your hints about the missing "unlock"s. And sorry for causing
you extra work by having the fix in a separate patch.
The patch you refer to ("power: supply: rt5033_charger: Add cable
detection and USB OTG supply") has its own history. It was already
applied once, showed up in linux-next, caused some issues, was therefore
removed again. In the meantime, some fixes were provided by different
contributors.
This series actually tries to apply that patch again, accompanied by two
fixes – and two additional clean-up patches. I added the fixes patches
as-is, also to credit the contributors.
Possibly the cover sheet of the series was a bit too thin about that
history.
Kind regards,
Jakob
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/5] power: supply: rt5033_charger: fix missing unlock
2023-09-28 19:46 ` Jakob Hauser
@ 2023-09-30 19:45 ` Sebastian Reichel
0 siblings, 0 replies; 11+ messages in thread
From: Sebastian Reichel @ 2023-09-30 19:45 UTC (permalink / raw)
To: Jakob Hauser
Cc: Marion & Christophe JAILLET, Lee Jones, Stephen Rothwell,
Yang Yingliang, Stephan Gerhold, Raymond Hackley, Henrik Grimler,
linux-pm, linux-kernel, ~postmarketos/upstreaming
[-- Attachment #1: Type: text/plain, Size: 1306 bytes --]
Hi,
On Thu, Sep 28, 2023 at 09:46:33PM +0200, Jakob Hauser wrote:
> On 28.09.23 07:23, Marion & Christophe JAILLET wrote:
> > Ok, but why not already in patch #1?
>
> Thanks for your hints about the missing "unlock"s. And sorry for causing you
> extra work by having the fix in a separate patch.
>
> The patch you refer to ("power: supply: rt5033_charger: Add cable detection
> and USB OTG supply") has its own history. It was already applied once,
> showed up in linux-next, caused some issues, was therefore removed again. In
> the meantime, some fixes were provided by different contributors.
Since the commit has been dropped, please merge the fixes into the
patch. E.g. patch #1 does not make any sense on its own.
> This series actually tries to apply that patch again, accompanied by two
> fixes – and two additional clean-up patches. I added the fixes patches
> as-is, also to credit the contributors.
You can use Co-developed-by tag for that.
> Possibly the cover sheet of the series was a bit too thin about that
> history.
>
> Kind regards,
> Jakob
Patches 4-5 look fine to me, but do not apply without 1-3. For 1-3 I
did not check them in detail. Please merge them first, since it's
quite hard to read in the current state.
Thanks,
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: (subset) [PATCH 0/5] Remaining patches for RT5033 charger
2023-09-27 20:25 ` [PATCH 0/5] Remaining patches for RT5033 charger Jakob Hauser
` (4 preceding siblings ...)
2023-09-27 20:26 ` [PATCH 5/5] power: supply: rt5033_charger: Replace "&pdev->dev" by "charger->dev" in probe Jakob Hauser
@ 2023-10-04 22:15 ` Sebastian Reichel
5 siblings, 0 replies; 11+ messages in thread
From: Sebastian Reichel @ 2023-10-04 22:15 UTC (permalink / raw)
To: Sebastian Reichel, Jakob Hauser
Cc: Lee Jones, Stephen Rothwell, Yang Yingliang, Christophe Jaillet,
Stephan Gerhold, Raymond Hackley, Henrik Grimler, linux-pm,
linux-kernel, ~postmarketos/upstreaming
On Wed, 27 Sep 2023 22:25:57 +0200, Jakob Hauser wrote:
> This series collects remaining patches for the RT5033 charger driver.
>
> The patches are based on 6.6-rc1.
>
> Mailing list links where the patches were taken from:
> - Patch 1
> https://lore.kernel.org/linux-next/20230821125741.3a2474d7@canb.auug.org.au
> - Patch 2
> https://lore.kernel.org/linux-pm/cover.1684182964.git.jahau@rocketmail.com/T/#m27164a05bbe2ec649be1fc32157b34ba814ff31f
> - Patch 3
> https://lore.kernel.org/linux-pm/20230822030207.644738-1-yangyingliang@huawei.com/T/#u
> - Patches 4 & 5
> https://lore.kernel.org/linux-pm/cover.1686948074.git.jahau@rocketmail.com/T/#u
>
> [...]
Applied, thanks!
[4/5] power: supply: rt5033_charger: Simplify initialization of rt5033_charger_data
commit: 1c6877f1768a34c04e3a82f9f950f78488a1753b
Best regards,
--
Sebastian Reichel <sebastian.reichel@collabora.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-10-04 22:15 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1695844349.git.jahau.ref@rocketmail.com>
2023-09-27 20:25 ` [PATCH 0/5] Remaining patches for RT5033 charger Jakob Hauser
2023-09-27 20:25 ` [PATCH 1/5] fixup for "power: Explicitly include correct DT includes" Jakob Hauser
2023-09-27 20:25 ` [PATCH 2/5] power: supply: rt5033_charger: Add cable detection and USB OTG supply Jakob Hauser
2023-09-28 5:21 ` Marion & Christophe JAILLET
2023-09-27 20:26 ` [PATCH 3/5] power: supply: rt5033_charger: fix missing unlock Jakob Hauser
2023-09-28 5:23 ` Marion & Christophe JAILLET
2023-09-28 19:46 ` Jakob Hauser
2023-09-30 19:45 ` Sebastian Reichel
2023-09-27 20:26 ` [PATCH 4/5] power: supply: rt5033_charger: Simplify initialization of rt5033_charger_data Jakob Hauser
2023-09-27 20:26 ` [PATCH 5/5] power: supply: rt5033_charger: Replace "&pdev->dev" by "charger->dev" in probe Jakob Hauser
2023-10-04 22:15 ` (subset) [PATCH 0/5] Remaining patches for RT5033 charger Sebastian Reichel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).