I noticed some further possible cleanups when reviewing
and applying Ricardo's patch to make power_supply_class
constant.
Signed-off-by: Sebastian Reichel <[email protected]>
---
Sebastian Reichel (2):
power: supply: core: add power_supply_for_each_device()
power: supply: core: simplify power_supply_class_init
drivers/power/supply/ab8500_btemp.c | 3 +--
drivers/power/supply/ab8500_chargalg.c | 3 +--
drivers/power/supply/ab8500_charger.c | 3 +--
drivers/power/supply/ab8500_fg.c | 3 +--
drivers/power/supply/apm_power.c | 3 +--
drivers/power/supply/power_supply_core.c | 43 +++++++++++++-------------------
include/linux/power_supply.h | 3 +--
7 files changed, 23 insertions(+), 38 deletions(-)
---
base-commit: 71c2cc5cbf686c2397f43cbcb51a31589bdcee7b
change-id: 20240301-psy-class-cleanup-6e6711d595aa
Best regards,
--
Sebastian Reichel <[email protected]>
Introduce power_supply_for_each_device(), which is a wrapper
for class_for_each_device() using the power_supply_class and
going through all devices.
This allows making the power_supply_class itself a local
variable, so that drivers cannot mess with it and simplifies
the code slightly.
Signed-off-by: Sebastian Reichel <[email protected]>
---
drivers/power/supply/ab8500_btemp.c | 3 +--
drivers/power/supply/ab8500_chargalg.c | 3 +--
drivers/power/supply/ab8500_charger.c | 3 +--
drivers/power/supply/ab8500_fg.c | 3 +--
drivers/power/supply/apm_power.c | 3 +--
drivers/power/supply/power_supply_core.c | 34 +++++++++++++++-----------------
include/linux/power_supply.h | 3 +--
7 files changed, 22 insertions(+), 30 deletions(-)
diff --git a/drivers/power/supply/ab8500_btemp.c b/drivers/power/supply/ab8500_btemp.c
index 41dba40fffdf..56f136b2d071 100644
--- a/drivers/power/supply/ab8500_btemp.c
+++ b/drivers/power/supply/ab8500_btemp.c
@@ -617,8 +617,7 @@ static int ab8500_btemp_get_ext_psy_data(struct device *dev, void *data)
*/
static void ab8500_btemp_external_power_changed(struct power_supply *psy)
{
- class_for_each_device(&power_supply_class, NULL, psy,
- ab8500_btemp_get_ext_psy_data);
+ power_supply_for_each_device(psy, ab8500_btemp_get_ext_psy_data);
}
/* ab8500 btemp driver interrupts and their respective isr */
diff --git a/drivers/power/supply/ab8500_chargalg.c b/drivers/power/supply/ab8500_chargalg.c
index 329ae784a72d..55ab7a28056e 100644
--- a/drivers/power/supply/ab8500_chargalg.c
+++ b/drivers/power/supply/ab8500_chargalg.c
@@ -1231,8 +1231,7 @@ static void ab8500_chargalg_algorithm(struct ab8500_chargalg *di)
int ret;
/* Collect data from all power_supply class devices */
- class_for_each_device(&power_supply_class, NULL,
- di->chargalg_psy, ab8500_chargalg_get_ext_psy_data);
+ power_supply_for_each_device(di->chargalg_psy, ab8500_chargalg_get_ext_psy_data);
ab8500_chargalg_end_of_charge(di);
ab8500_chargalg_check_temp(di);
diff --git a/drivers/power/supply/ab8500_charger.c b/drivers/power/supply/ab8500_charger.c
index 1c2b69bbed17..9b34d1a60f66 100644
--- a/drivers/power/supply/ab8500_charger.c
+++ b/drivers/power/supply/ab8500_charger.c
@@ -1949,8 +1949,7 @@ static void ab8500_charger_check_vbat_work(struct work_struct *work)
struct ab8500_charger *di = container_of(work,
struct ab8500_charger, check_vbat_work.work);
- class_for_each_device(&power_supply_class, NULL,
- &di->usb_chg, ab8500_charger_get_ext_psy_data);
+ power_supply_for_each_device(&di->usb_chg, ab8500_charger_get_ext_psy_data);
/* First run old_vbat is 0. */
if (di->old_vbat == 0)
diff --git a/drivers/power/supply/ab8500_fg.c b/drivers/power/supply/ab8500_fg.c
index e49e704023e1..2ccaf6116c09 100644
--- a/drivers/power/supply/ab8500_fg.c
+++ b/drivers/power/supply/ab8500_fg.c
@@ -2407,8 +2407,7 @@ static int ab8500_fg_init_hw_registers(struct ab8500_fg *di)
*/
static void ab8500_fg_external_power_changed(struct power_supply *psy)
{
- class_for_each_device(&power_supply_class, NULL, psy,
- ab8500_fg_get_ext_psy_data);
+ power_supply_for_each_device(psy, ab8500_fg_get_ext_psy_data);
}
/**
diff --git a/drivers/power/supply/apm_power.c b/drivers/power/supply/apm_power.c
index 034f28699977..8ef1b6f1f787 100644
--- a/drivers/power/supply/apm_power.c
+++ b/drivers/power/supply/apm_power.c
@@ -79,8 +79,7 @@ static void find_main_battery(void)
main_battery = NULL;
bp.main = main_battery;
- error = class_for_each_device(&power_supply_class, NULL, &bp,
- __find_main_battery);
+ error = power_supply_for_each_device(&bp, __find_main_battery);
if (error) {
main_battery = bp.main;
return;
diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 4f27f17f8741..0eb8a57dda70 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -25,12 +25,10 @@
#include "power_supply.h"
#include "samsung-sdi-battery.h"
-/* exported for the APM Power driver, APM emulation */
-const struct class power_supply_class = {
+static const struct class power_supply_class = {
.name = "power_supply",
.dev_uevent = power_supply_uevent,
};
-EXPORT_SYMBOL_GPL(power_supply_class);
static BLOCKING_NOTIFIER_HEAD(power_supply_notifier);
@@ -100,8 +98,7 @@ static void power_supply_changed_work(struct work_struct *work)
if (likely(psy->changed)) {
psy->changed = false;
spin_unlock_irqrestore(&psy->changed_lock, flags);
- class_for_each_device(&power_supply_class, NULL, psy,
- __power_supply_changed_work);
+ power_supply_for_each_device(psy, __power_supply_changed_work);
power_supply_update_leds(psy);
blocking_notifier_call_chain(&power_supply_notifier,
PSY_EVENT_PROP_CHANGED, psy);
@@ -119,6 +116,12 @@ static void power_supply_changed_work(struct work_struct *work)
spin_unlock_irqrestore(&psy->changed_lock, flags);
}
+int power_supply_for_each_device(void *data, int (*fn)(struct device *dev, void *data))
+{
+ return class_for_each_device(&power_supply_class, NULL, data, fn);
+}
+EXPORT_SYMBOL_GPL(power_supply_for_each_device);
+
void power_supply_changed(struct power_supply *psy)
{
unsigned long flags;
@@ -194,8 +197,7 @@ static int power_supply_populate_supplied_from(struct power_supply *psy)
{
int error;
- error = class_for_each_device(&power_supply_class, NULL, psy,
- __power_supply_populate_supplied_from);
+ error = power_supply_for_each_device(psy, __power_supply_populate_supplied_from);
dev_dbg(&psy->dev, "%s %d\n", __func__, error);
@@ -208,7 +210,7 @@ static int __power_supply_find_supply_from_node(struct device *dev,
struct device_node *np = data;
struct power_supply *epsy = dev_get_drvdata(dev);
- /* returning non-zero breaks out of class_for_each_device loop */
+ /* returning non-zero breaks out of power_supply_for_each_device loop */
if (epsy->of_node == np)
return 1;
@@ -220,17 +222,16 @@ static int power_supply_find_supply_from_node(struct device_node *supply_node)
int error;
/*
- * class_for_each_device() either returns its own errors or values
+ * power_supply_for_each_device() either returns its own errors or values
* returned by __power_supply_find_supply_from_node().
*
* __power_supply_find_supply_from_node() will return 0 (no match)
* or 1 (match).
*
- * We return 0 if class_for_each_device() returned 1, -EPROBE_DEFER if
+ * We return 0 if power_supply_for_each_device() returned 1, -EPROBE_DEFER if
* it returned 0, or error as returned by it.
*/
- error = class_for_each_device(&power_supply_class, NULL, supply_node,
- __power_supply_find_supply_from_node);
+ error = power_supply_for_each_device(supply_node, __power_supply_find_supply_from_node);
return error ? (error == 1 ? 0 : error) : -EPROBE_DEFER;
}
@@ -336,8 +337,7 @@ int power_supply_am_i_supplied(struct power_supply *psy)
struct psy_am_i_supplied_data data = { psy, 0 };
int error;
- error = class_for_each_device(&power_supply_class, NULL, &data,
- __power_supply_am_i_supplied);
+ error = power_supply_for_each_device(&data, __power_supply_am_i_supplied);
dev_dbg(&psy->dev, "%s count %u err %d\n", __func__, data.count, error);
@@ -372,8 +372,7 @@ int power_supply_is_system_supplied(void)
int error;
unsigned int count = 0;
- error = class_for_each_device(&power_supply_class, NULL, &count,
- __power_supply_is_system_supplied);
+ error = power_supply_for_each_device(&count, __power_supply_is_system_supplied);
/*
* If no system scope power class device was found at all, most probably we
@@ -419,8 +418,7 @@ int power_supply_get_property_from_supplier(struct power_supply *psy,
* This function is not intended for use with a supply with multiple
* suppliers, we simply pick the first supply to report the psp.
*/
- ret = class_for_each_device(&power_supply_class, NULL, &data,
- __power_supply_get_supplier_property);
+ ret = power_supply_for_each_device(&data, __power_supply_get_supplier_property);
if (ret < 0)
return ret;
if (ret == 0)
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 514f652de64d..92dd205774ec 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -894,8 +894,7 @@ extern int power_supply_powers(struct power_supply *psy, struct device *dev);
#define to_power_supply(device) container_of(device, struct power_supply, dev)
extern void *power_supply_get_drvdata(struct power_supply *psy);
-/* For APM emulation, think legacy userspace. */
-extern const struct class power_supply_class;
+extern int power_supply_for_each_device(void *data, int (*fn)(struct device *dev, void *data));
static inline bool power_supply_is_amp_property(enum power_supply_property psp)
{
--
2.43.0
Technically the sysfs attributes should be initialized
before the class is registered, since that will use them.
As a nice side effect this nicely simplifies the code,
since it allows dropping the helper variable.
Signed-off-by: Sebastian Reichel <[email protected]>
---
drivers/power/supply/power_supply_core.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 0eb8a57dda70..4daea6ed8f1d 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -1622,15 +1622,8 @@ EXPORT_SYMBOL_GPL(power_supply_get_drvdata);
static int __init power_supply_class_init(void)
{
- int err;
-
- err = class_register(&power_supply_class);
- if (err)
- return err;
-
power_supply_init_attrs();
-
- return 0;
+ return class_register(&power_supply_class);
}
static void __exit power_supply_class_exit(void)
--
2.43.0
On 1 Mar 23:58, Sebastian Reichel wrote:
> I noticed some further possible cleanups when reviewing
> and applying Ricardo's patch to make power_supply_class
> constant.
Good one!
Reviewed-by: Ricardo B. Marliere <[email protected]>
Thank you,
- Ricardo.
>
> Signed-off-by: Sebastian Reichel <[email protected]>
> ---
> Sebastian Reichel (2):
> power: supply: core: add power_supply_for_each_device()
> power: supply: core: simplify power_supply_class_init
>
> drivers/power/supply/ab8500_btemp.c | 3 +--
> drivers/power/supply/ab8500_chargalg.c | 3 +--
> drivers/power/supply/ab8500_charger.c | 3 +--
> drivers/power/supply/ab8500_fg.c | 3 +--
> drivers/power/supply/apm_power.c | 3 +--
> drivers/power/supply/power_supply_core.c | 43 +++++++++++++-------------------
> include/linux/power_supply.h | 3 +--
> 7 files changed, 23 insertions(+), 38 deletions(-)
> ---
> base-commit: 71c2cc5cbf686c2397f43cbcb51a31589bdcee7b
> change-id: 20240301-psy-class-cleanup-6e6711d595aa
>
> Best regards,
> --
> Sebastian Reichel <[email protected]>
>
On Fri, 01 Mar 2024 23:58:25 +0100, Sebastian Reichel wrote:
> I noticed some further possible cleanups when reviewing
> and applying Ricardo's patch to make power_supply_class
> constant.
>
>
Applied, thanks!
[1/2] power: supply: core: add power_supply_for_each_device()
commit: 68ade0976df7979eac5f1d46320ff798f5043af6
[2/2] power: supply: core: simplify power_supply_class_init
commit: ea1ec769d1f01a9900127e83e63dfdd77d096c8a
Best regards,
--
Sebastian Reichel <[email protected]>