power: supply: rt5033_charger: Simplify initialization of rt5033_charger_data
A message that is not in the archive (sent off-list or not yet received): [email protected]
cover Clean-up RT5033 charger "rt5033_charger_data" and probe "&pdev->dev"
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/[email protected]/T/#u
[2] https://lore.kernel.org/linux-pm/[email protected]/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(-)
Signature · 1 line
2.39.2
PATCH 1/2 power: supply: rt5033_charger: Simplify initialization of rt5033_charger_data
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.
--- drivers/power/supply/rt5033_charger.c | 29 +++++++++++---------------- 1 file changed, 12 insertions(+), 17 deletions(-)
drivers/power/supply/rt5033_charger.c +12 -17
@@ -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)
Signature · 1 line
2.39.2
PATCH 2/2 power: supply: rt5033_charger: Replace "&pdev->dev" by "charger->dev" in probe
In reply to Jakob HauserAt 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".
--- drivers/power/supply/rt5033_charger.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
drivers/power/supply/rt5033_charger.c +7 -7
@@ -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:
Signature · 1 line
2.39.2