[PATCH 0/2] Clean-up RT5033 charger "rt5033_charger_data" and probe "&pdev->dev"

[PATCH 0/2] Clean-up RT5033 charger "rt5033_charger_data" and probe "&pdev->dev"

[Jakob Hauser]

During the review of the series "Add RT5033 charger device driver" [1] I
noticed two things I'd like to clean up. Additionally, in v5 of that series,
Christophe Jaillet pointed out an unclean error handling of function
"rt5033_charger_data" [2]. The clean-up solves this as well. As I didn't want
to disturb the review process of the series in its late phase, I now add these
clean-ups as a follow-up series.

I was a bit surprised about the patchset "Add RT5033 charger device driver"
being added via the mfd subsystem. This clean-up series affects the charger
driver, which is actually part of the power supply subsystem. I'm not
fully sure what the series should be based on and how it will be applied
best.

Additionally I just realized that in the immutable branch "ib-mfd-power-v6.5"
of kernel/git/lee/mfd.git one patch of the previous series is missing. Because
of this, I currently can't use linux-next or mfd.git as a base. Instead I based
this clean-up patchset right on top of the previous patchset. An instance of
that base can be seen on my GitHub page [3][4].

[1] https://lore.kernel.org/linux-pm/cover.1684182964.git.jahau@rocketmail.com/T/#u
[2] https://lore.kernel.org/linux-pm/20230514123130.41172-1-jahau@rocketmail.com/T/#mbe33ab528f3d8ae136f01b13f45a9be35c0ad061
[3] https://github.com/Jakko3/linux/commits/rt5033-charger_v6
[4] https://github.com/Jakko3/linux/blob/rt5033-charger_v6/drivers/power/supply/rt5033_charger.c

Jakob Hauser (2):
  power: supply: rt5033_charger: Simplify initialization of
    rt5033_charger_data
  power: supply: rt5033_charger: Replace "&pdev->dev" by "charger->dev"
    in probe

 drivers/power/supply/rt5033_charger.c | 43 ++++++++++++---------------
 1 file changed, 19 insertions(+), 24 deletions(-)

-- 
2.39.2

[PATCH 1/2] power: supply: rt5033_charger: Simplify initialization of rt5033_charger_data

[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 e0303ca5a8db..361a36c2a195 100644
--- a/drivers/power/supply/rt5033_charger.c
+++ b/drivers/power/supply/rt5033_charger.c
@@ -28,7 +28,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;
@@ -130,7 +130,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;
@@ -204,7 +204,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;
@@ -249,7 +249,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;
@@ -539,21 +539,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;
@@ -562,7 +557,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)
@@ -679,9 +674,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

[PATCH 2/2] power: supply: rt5033_charger: Replace "&pdev->dev" by "charger->dev" in probe

[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 361a36c2a195..71a237cd1d61 100644
--- a/drivers/power/supply/rt5033_charger.c
+++ b/drivers/power/supply/rt5033_charger.c
@@ -667,11 +667,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);
@@ -690,22 +690,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