From 5d8a4219a0795a321606c51582898223db80e874 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Tue, 24 Feb 2015 15:33:50 +1100 Subject: power_supply core: support use of devres to register/unregister a power supply. Using devm_power_supply_register allows the unregister to happen automatically on error or final put. Signed-off-by: NeilBrown Signed-off-by: Sebastian Reichel --- include/linux/power_supply.h | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'include/linux') diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h index 096dbced02ac..f606d6b4bd56 100644 --- a/include/linux/power_supply.h +++ b/include/linux/power_supply.h @@ -278,6 +278,10 @@ extern int power_supply_register(struct device *parent, struct power_supply *psy); extern int power_supply_register_no_ws(struct device *parent, struct power_supply *psy); +extern int devm_power_supply_register(struct device *parent, + struct power_supply *psy); +extern int devm_power_supply_register_no_ws(struct device *parent, + struct power_supply *psy); extern void power_supply_unregister(struct power_supply *psy); extern int power_supply_powers(struct power_supply *psy, struct device *dev); -- cgit v1.2.3 From e44ea364394499d38a26ed4c9668fb378ae8797f Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Thu, 12 Mar 2015 08:44:01 +0100 Subject: power_supply: Add driver private data Allow drivers to store private data inside power_supply structure for later usage in power supply operations. Usage of driver private data is necessary to access driver's state container object from power supply calls (like get_property()) if struct 'power_supply' is a stored there as a pointer, for example: struct some_driver_info { struct i2c_client *client; struct power_supply *power_supply; ... } In such case one cannot use container_of() and must store pointer to state container as private data. Signed-off-by: Krzysztof Kozlowski Reviewed-by: Bartlomiej Zolnierkiewicz Reviewed-by: Sebastian Reichel Acked-by: Pavel Machek Signed-off-by: Sebastian Reichel --- include/linux/power_supply.h | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'include/linux') diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h index f606d6b4bd56..e30d85c0158d 100644 --- a/include/linux/power_supply.h +++ b/include/linux/power_supply.h @@ -209,6 +209,9 @@ struct power_supply { /* For APM emulation, think legacy userspace. */ int use_for_apm; + /* Driver private data */ + void *drv_data; + /* private */ struct device *dev; struct work_struct changed_work; @@ -285,6 +288,7 @@ extern int devm_power_supply_register_no_ws(struct device *parent, extern void power_supply_unregister(struct power_supply *psy); extern int power_supply_powers(struct power_supply *psy, struct device *dev); +extern void *power_supply_get_drvdata(struct power_supply *psy); /* For APM emulation, think legacy userspace. */ extern struct class *power_supply_class; -- cgit v1.2.3 From 2dc9215d7c94f7f9f34ccf8b1710ad73d82f6216 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Thu, 12 Mar 2015 08:44:02 +0100 Subject: power_supply: Move run-time configuration to separate structure Add new structure 'power_supply_config' for holding run-time initialization data like of_node, supplies and private driver data. The power_supply_register() function is changed so all power supply drivers need updating. When registering the power supply this new 'power_supply_config' should be used instead of directly initializing 'struct power_supply'. This allows changing the ownership of power_supply structure from driver to the power supply core in next patches. When a driver does not use of_node or supplies then it should use NULL as config. If driver uses of_node or supplies then it should allocate config on stack and initialize it with proper values. Signed-off-by: Krzysztof Kozlowski Reviewed-by: Bartlomiej Zolnierkiewicz Acked-by: Pavel Machek [for the nvec part] Reviewed-by: Marc Dietrich [for drivers/platform/x86/compal-laptop.c] Reviewed-by: Darren Hart [for drivers/hid/*] Reviewed-by: Jiri Kosina Signed-off-by: Sebastian Reichel --- include/linux/power_supply.h | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) (limited to 'include/linux') diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h index e30d85c0158d..0d7c95f634a5 100644 --- a/include/linux/power_supply.h +++ b/include/linux/power_supply.h @@ -176,6 +176,16 @@ union power_supply_propval { struct device; struct device_node; +/* Power supply instance specific configuration */ +struct power_supply_config { + struct device_node *of_node; + /* Driver private data */ + void *drv_data; + + char **supplied_to; + size_t num_supplicants; +}; + struct power_supply { const char *name; enum power_supply_type type; @@ -278,13 +288,17 @@ static inline int power_supply_is_system_supplied(void) { return -ENOSYS; } #endif extern int power_supply_register(struct device *parent, - struct power_supply *psy); + struct power_supply *psy, + const struct power_supply_config *cfg); extern int power_supply_register_no_ws(struct device *parent, - struct power_supply *psy); + struct power_supply *psy, + const struct power_supply_config *cfg); extern int devm_power_supply_register(struct device *parent, - struct power_supply *psy); + struct power_supply *psy, + const struct power_supply_config *cfg); extern int devm_power_supply_register_no_ws(struct device *parent, - struct power_supply *psy); + struct power_supply *psy, + const struct power_supply_config *cfg); extern void power_supply_unregister(struct power_supply *psy); extern int power_supply_powers(struct power_supply *psy, struct device *dev); -- cgit v1.2.3 From bc1540561c9ede1efb6d7bf44804676d3d02a3cc Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Thu, 12 Mar 2015 08:44:03 +0100 Subject: power_supply: Add API for safe access of power supply function attrs Add simple wrappers for accessing power supply's function attributes: - get_property -> power_supply_get_property - set_property -> power_supply_set_property - property_is_writeable -> power_supply_property_is_writeable - external_power_changed -> power_supply_external_power_changed This API along with atomic usage counter adds a safe way of accessing a power supply from another driver. If power supply is unregistered after obtaining reference to it by some driver, then the API wrappers won't be executed in invalid (freed) context. Next patch changing the ownership of power supply class is still needed to fully fix race conditions in accessing freed power supply. Signed-off-by: Krzysztof Kozlowski Reviewed-by: Bartlomiej Zolnierkiewicz Reviewed-by: Sebastian Reichel Acked-by: Pavel Machek Signed-off-by: Sebastian Reichel --- include/linux/power_supply.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'include/linux') diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h index 0d7c95f634a5..7ae60346465f 100644 --- a/include/linux/power_supply.h +++ b/include/linux/power_supply.h @@ -199,6 +199,12 @@ struct power_supply { size_t num_supplies; struct device_node *of_node; + /* + * Functions for drivers implementing power supply class. + * These shouldn't be called directly by other drivers for accessing + * this power supply. Instead use power_supply_*() functions (for + * example power_supply_get_property()). + */ int (*get_property)(struct power_supply *psy, enum power_supply_property psp, union power_supply_propval *val); @@ -227,6 +233,7 @@ struct power_supply { struct work_struct changed_work; spinlock_t changed_lock; bool changed; + atomic_t use_cnt; #ifdef CONFIG_THERMAL struct thermal_zone_device *tzd; struct thermal_cooling_device *tcd; @@ -287,6 +294,15 @@ extern int power_supply_is_system_supplied(void); static inline int power_supply_is_system_supplied(void) { return -ENOSYS; } #endif +extern int power_supply_get_property(struct power_supply *psy, + enum power_supply_property psp, + union power_supply_propval *val); +extern int power_supply_set_property(struct power_supply *psy, + enum power_supply_property psp, + const union power_supply_propval *val); +extern int power_supply_property_is_writeable(struct power_supply *psy, + enum power_supply_property psp); +extern void power_supply_external_power_changed(struct power_supply *psy); extern int power_supply_register(struct device *parent, struct power_supply *psy, const struct power_supply_config *cfg); -- cgit v1.2.3 From 297d716f6260cc9421d971b124ca196b957ee458 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Thu, 12 Mar 2015 08:44:11 +0100 Subject: power_supply: Change ownership from driver to core Change the ownership of power_supply structure from each driver implementing the class to the power supply core. The patch changes power_supply_register() function thus all drivers implementing power supply class are adjusted. Each driver provides the implementation of power supply. However it should not be the owner of power supply class instance because it is exposed by core to other subsystems with power_supply_get_by_name(). These other subsystems have no knowledge when the driver will unregister the power supply. This leads to several issues when driver is unbound - mostly because user of power supply accesses freed memory. Instead let the core own the instance of struct 'power_supply'. Other users of this power supply will still access valid memory because it will be freed when device reference count reaches 0. Currently this means "it will leak" but power_supply_put() call in next patches will solve it. This solves invalid memory references in following race condition scenario: Thread 1: charger manager Thread 2: power supply driver, used by charger manager THREAD 1 (charger manager) THREAD 2 (power supply driver) ========================== ============================== psy = power_supply_get_by_name() Driver unbind, .remove power_supply_unregister() Device fully removed psy->get_property() The 'get_property' call is executed in invalid context because the driver was unbound and struct 'power_supply' memory was freed. This could be observed easily with charger manager driver (here compiled with max17040 fuel gauge): $ cat /sys/devices/virtual/power_supply/cm-battery/capacity & $ echo "1-0036" > /sys/bus/i2c/drivers/max17040/unbind [ 55.725123] Unable to handle kernel NULL pointer dereference at virtual address 00000000 [ 55.732584] pgd = d98d4000 [ 55.734060] [00000000] *pgd=5afa2831, *pte=00000000, *ppte=00000000 [ 55.740318] Internal error: Oops: 80000007 [#1] PREEMPT SMP ARM [ 55.746210] Modules linked in: [ 55.749259] CPU: 1 PID: 2936 Comm: cat Tainted: G W 3.19.0-rc1-next-20141226-00048-gf79f475f3c44-dirty #1496 [ 55.760190] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) [ 55.766270] task: d9b76f00 ti: daf54000 task.ti: daf54000 [ 55.771647] PC is at 0x0 [ 55.774182] LR is at charger_get_property+0x2f4/0x36c [ 55.779201] pc : [<00000000>] lr : [] psr: 60000013 [ 55.779201] sp : daf55e90 ip : 00000003 fp : 00000000 [ 55.790657] r10: 00000000 r9 : c06e2878 r8 : d9b26c68 [ 55.795865] r7 : dad81610 r6 : daec7410 r5 : daf55ebc r4 : 00000000 [ 55.802367] r3 : 00000000 r2 : daf55ebc r1 : 0000002a r0 : d9b26c68 [ 55.808879] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user [ 55.815994] Control: 10c5387d Table: 598d406a DAC: 00000015 [ 55.821723] Process cat (pid: 2936, stack limit = 0xdaf54210) [ 55.827451] Stack: (0xdaf55e90 to 0xdaf56000) [ 55.831795] 5e80: 60000013 c01459c4 0000002a c06f8ef8 [ 55.839956] 5ea0: db651000 c06f8ef8 daebac00 c04cb668 daebac08 c0346864 00000000 c01459c4 [ 55.848115] 5ec0: d99eaa80 c06f8ef8 00000fff 00001000 db651000 c027f25c c027f240 d99eaa80 [ 55.856274] 5ee0: d9a06c00 c0146218 daf55f18 00001000 d99eaa80 db4c18c0 00000001 00000001 [ 55.864468] 5f00: daf55f80 c0144c78 c0144c54 c0107f90 00015000 d99eaab0 00000000 00000000 [ 55.872603] 5f20: 000051c7 00000000 db4c18c0 c04a9370 00015000 00001000 daf55f80 00001000 [ 55.880763] 5f40: daf54000 00015000 00000000 c00e53dc db4c18c0 c00e548c 0000000d 00008124 [ 55.888937] 5f60: 00000001 00000000 00000000 db4c18c0 db4c18c0 00001000 00015000 c00e5550 [ 55.897099] 5f80: 00000000 00000000 00001000 00001000 00015000 00000003 00000003 c000f364 [ 55.905239] 5fa0: 00000000 c000f1a0 00001000 00015000 00000003 00015000 00001000 0001333c [ 55.913399] 5fc0: 00001000 00015000 00000003 00000003 00000002 00000000 00000000 00000000 [ 55.921560] 5fe0: 7fffe000 be999850 0000a225 b6f3c19c 60000010 00000003 00000000 00000000 [ 55.929744] [] (charger_get_property) from [] (power_supply_show_property+0x48/0x20c) [ 55.939286] [] (power_supply_show_property) from [] (dev_attr_show+0x1c/0x48) [ 55.948130] [] (dev_attr_show) from [] (sysfs_kf_seq_show+0x84/0x104) [ 55.956298] [] (sysfs_kf_seq_show) from [] (kernfs_seq_show+0x24/0x28) [ 55.964536] [] (kernfs_seq_show) from [] (seq_read+0x1b0/0x484) [ 55.972172] [] (seq_read) from [] (__vfs_read+0x18/0x4c) [ 55.979188] [] (__vfs_read) from [] (vfs_read+0x7c/0x100) [ 55.986304] [] (vfs_read) from [] (SyS_read+0x40/0x8c) [ 55.993164] [] (SyS_read) from [] (ret_fast_syscall+0x0/0x48) [ 56.000626] Code: bad PC value [ 56.011652] ---[ end trace 7b64343fbdae8ef1 ]--- Signed-off-by: Krzysztof Kozlowski Reviewed-by: Bartlomiej Zolnierkiewicz [for the nvec part] Reviewed-by: Marc Dietrich [for compal-laptop.c] Acked-by: Darren Hart [for the mfd part] Acked-by: Lee Jones [for the hid part] Acked-by: Jiri Kosina [for the acpi part] Acked-by: Rafael J. Wysocki Signed-off-by: Sebastian Reichel --- include/linux/hid.h | 6 ++-- include/linux/mfd/abx500/ux500_chargalg.h | 11 +++++-- include/linux/mfd/rt5033.h | 2 +- include/linux/mfd/wm8350/supply.h | 6 ++-- include/linux/power/charger-manager.h | 3 +- include/linux/power_supply.h | 49 +++++++++++++++++++------------ 6 files changed, 47 insertions(+), 30 deletions(-) (limited to 'include/linux') diff --git a/include/linux/hid.h b/include/linux/hid.h index efc7787a41a8..f94cf28e4b7c 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -514,10 +514,10 @@ struct hid_device { /* device report descriptor */ #ifdef CONFIG_HID_BATTERY_STRENGTH /* * Power supply information for HID devices which report - * battery strength. power_supply is registered iff - * battery.name is non-NULL. + * battery strength. power_supply was successfully registered if + * battery is non-NULL. */ - struct power_supply battery; + struct power_supply *battery; __s32 battery_min; __s32 battery_max; __s32 battery_report_type; diff --git a/include/linux/mfd/abx500/ux500_chargalg.h b/include/linux/mfd/abx500/ux500_chargalg.h index 234c99143bf7..67703f23e7ba 100644 --- a/include/linux/mfd/abx500/ux500_chargalg.h +++ b/include/linux/mfd/abx500/ux500_chargalg.h @@ -9,8 +9,13 @@ #include -#define psy_to_ux500_charger(x) container_of((x), \ - struct ux500_charger, psy) +/* + * Valid only for supplies of type: + * - POWER_SUPPLY_TYPE_MAINS, + * - POWER_SUPPLY_TYPE_USB, + * because only them store as drv_data pointer to struct ux500_charger. + */ +#define psy_to_ux500_charger(x) power_supply_get_drvdata(psy) /* Forward declaration */ struct ux500_charger; @@ -35,7 +40,7 @@ struct ux500_charger_ops { * @power_path USB power path support */ struct ux500_charger { - struct power_supply psy; + struct power_supply *psy; struct ux500_charger_ops ops; int max_out_volt; int max_out_curr; diff --git a/include/linux/mfd/rt5033.h b/include/linux/mfd/rt5033.h index 010cff49a98e..6cff5cf458d2 100644 --- a/include/linux/mfd/rt5033.h +++ b/include/linux/mfd/rt5033.h @@ -39,7 +39,7 @@ struct rt5033_battery { struct i2c_client *client; struct rt5033_dev *rt5033; struct regmap *regmap; - struct power_supply psy; + struct power_supply *psy; }; /* RT5033 charger platform data */ diff --git a/include/linux/mfd/wm8350/supply.h b/include/linux/mfd/wm8350/supply.h index 2b9479310bbd..8dc93673e34a 100644 --- a/include/linux/mfd/wm8350/supply.h +++ b/include/linux/mfd/wm8350/supply.h @@ -123,9 +123,9 @@ struct wm8350_charger_policy { struct wm8350_power { struct platform_device *pdev; - struct power_supply battery; - struct power_supply usb; - struct power_supply ac; + struct power_supply *battery; + struct power_supply *usb; + struct power_supply *ac; struct wm8350_charger_policy *policy; int rev_g_coeff; diff --git a/include/linux/power/charger-manager.h b/include/linux/power/charger-manager.h index 416ebeb6ee1e..eadf28cb2fc9 100644 --- a/include/linux/power/charger-manager.h +++ b/include/linux/power/charger-manager.h @@ -242,7 +242,8 @@ struct charger_manager { int emergency_stop; char psy_name_buf[PSY_NAME_MAX + 1]; - struct power_supply charger_psy; + struct power_supply_desc charger_psy_desc; + struct power_supply *charger_psy; u64 charging_start_time; u64 charging_end_time; diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h index 7ae60346465f..ea15eb68f609 100644 --- a/include/linux/power_supply.h +++ b/include/linux/power_supply.h @@ -13,6 +13,7 @@ #ifndef __LINUX_POWER_SUPPLY_H__ #define __LINUX_POWER_SUPPLY_H__ +#include #include #include #include @@ -173,10 +174,10 @@ union power_supply_propval { const char *strval; }; -struct device; struct device_node; +struct power_supply; -/* Power supply instance specific configuration */ +/* Run-time specific power supply configuration */ struct power_supply_config { struct device_node *of_node; /* Driver private data */ @@ -186,19 +187,13 @@ struct power_supply_config { size_t num_supplicants; }; -struct power_supply { +/* Description of power supply */ +struct power_supply_desc { const char *name; enum power_supply_type type; enum power_supply_property *properties; size_t num_properties; - char **supplied_to; - size_t num_supplicants; - - char **supplied_from; - size_t num_supplies; - struct device_node *of_node; - /* * Functions for drivers implementing power supply class. * These shouldn't be called directly by other drivers for accessing @@ -224,12 +219,23 @@ struct power_supply { bool no_thermal; /* For APM emulation, think legacy userspace. */ int use_for_apm; +}; + +struct power_supply { + const struct power_supply_desc *desc; + + char **supplied_to; + size_t num_supplicants; + + char **supplied_from; + size_t num_supplies; + struct device_node *of_node; /* Driver private data */ void *drv_data; /* private */ - struct device *dev; + struct device dev; struct work_struct changed_work; spinlock_t changed_lock; bool changed; @@ -303,17 +309,22 @@ extern int power_supply_set_property(struct power_supply *psy, extern int power_supply_property_is_writeable(struct power_supply *psy, enum power_supply_property psp); extern void power_supply_external_power_changed(struct power_supply *psy); -extern int power_supply_register(struct device *parent, - struct power_supply *psy, + +extern struct power_supply *__must_check +power_supply_register(struct device *parent, + const struct power_supply_desc *desc, const struct power_supply_config *cfg); -extern int power_supply_register_no_ws(struct device *parent, - struct power_supply *psy, +extern struct power_supply *__must_check +power_supply_register_no_ws(struct device *parent, + const struct power_supply_desc *desc, const struct power_supply_config *cfg); -extern int devm_power_supply_register(struct device *parent, - struct power_supply *psy, +extern struct power_supply *__must_check +devm_power_supply_register(struct device *parent, + const struct power_supply_desc *desc, const struct power_supply_config *cfg); -extern int devm_power_supply_register_no_ws(struct device *parent, - struct power_supply *psy, +extern struct power_supply *__must_check +devm_power_supply_register_no_ws(struct device *parent, + const struct power_supply_desc *desc, const struct power_supply_config *cfg); extern void power_supply_unregister(struct power_supply *psy); extern int power_supply_powers(struct power_supply *psy, struct device *dev); -- cgit v1.2.3 From 1a352462b5377ac68f5955d674b3460c7bac52a3 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Thu, 12 Mar 2015 08:44:12 +0100 Subject: power_supply: Add power_supply_put for decrementing device reference counter The power_supply_get_by_phandle() and power_supply_get_by_name() use function class_find_device() for obtaining the reference to power supply. Each use of class_find_device() increases the power supply's device reference counter. However the reference counter was not decreased by users of this API. Thus final device_unregister() call from power_supply_unregister() could not release the device and clean up its resources. This lead to memory leak if at least once power_supply_get_by_*() was called between registering and unregistering the power supply. Add and document new API power_supply_put() for decrementing the reference counter. Signed-off-by: Krzysztof Kozlowski Acked-by: Pavel Machek Reviewed-by: Bartlomiej Zolnierkiewicz Reviewed-by: Sebastian Reichel Signed-off-by: Sebastian Reichel --- include/linux/power_supply.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include/linux') diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h index ea15eb68f609..75a1dd8dc56e 100644 --- a/include/linux/power_supply.h +++ b/include/linux/power_supply.h @@ -282,6 +282,7 @@ extern struct atomic_notifier_head power_supply_notifier; extern int power_supply_reg_notifier(struct notifier_block *nb); extern void power_supply_unreg_notifier(struct notifier_block *nb); extern struct power_supply *power_supply_get_by_name(const char *name); +extern void power_supply_put(struct power_supply *psy); #ifdef CONFIG_OF extern struct power_supply *power_supply_get_by_phandle(struct device_node *np, const char *property); -- cgit v1.2.3 From 709c2c70c8ea73f488971f1de3c39fbdac995951 Mon Sep 17 00:00:00 2001 From: Beomho Seo Date: Fri, 3 Apr 2015 17:26:08 +0900 Subject: power: max17042_battery: Use reg type instead of chip type Currently, max17042 battery driver choose register map by MAX17042_DevName register. But it is return IC specific firmware version. So other maxim chip hard to use this drvier. This patch choose chip type from driver_data. Signed-off-by: Beomho Seo Reviewed-by: Krzysztof Kozlowski Signed-off-by: Sebastian Reichel --- include/linux/power/max17042_battery.h | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'include/linux') diff --git a/include/linux/power/max17042_battery.h b/include/linux/power/max17042_battery.h index 89dd84f47c6e..cf112b4075c8 100644 --- a/include/linux/power/max17042_battery.h +++ b/include/linux/power/max17042_battery.h @@ -126,7 +126,14 @@ enum max17047_register { MAX17047_QRTbl30 = 0x42, }; -enum max170xx_chip_type {MAX17042, MAX17047}; +enum max170xx_chip_type { + MAXIM_DEVICE_TYPE_UNKNOWN = 0, + MAXIM_DEVICE_TYPE_MAX17042, + MAXIM_DEVICE_TYPE_MAX17047, + MAXIM_DEVICE_TYPE_MAX17050, + + MAXIM_DEVICE_TYPE_NUM +}; /* * used for setting a register to a desired value -- cgit v1.2.3