Add DT support for generic ADC battery
Subsystems (5)
Sebastian Reichel
maintainercover Add DT support for generic ADC battery
Hi,
This series cleans up the generic ADC battery driver and adds devicetree support. The plan is to use the driver to add upstream support for a handheld thermal camera.
Instead of reading and exposing the monitored battery data manually I started the series with an addition to the power-supply core, which allows automatic handling of the static battery information. It simplifies the generic-adc-battery driver a lot and should also be useful for other battery drivers.
-- Sebastian
Sebastian Reichel (11):
dt-bindings: power: supply: adc-battery: add binding
power: supply: core: auto-exposure of simple-battery data
power: supply: generic-adc-battery: convert to managed resources
power: supply: generic-adc-battery: fix unit scaling
power: supply: generic-adc-battery: drop jitter delay support
power: supply: generic-adc-battery: drop charge now support
power: supply: generic-adc-battery: drop memory alloc error message
power: supply: generic-adc-battery: use simple-battery API
power: supply: generic-adc-battery: simplify read_channel logic
power: supply: generic-adc-battery: add DT support
power: supply: generic-adc-battery: update copyright info
.../bindings/power/supply/adc-battery.yaml | 67 ++++++ drivers/power/supply/generic-adc-battery.c | 221 +++++------------- drivers/power/supply/power_supply_core.c | 153 ++++++++++-- drivers/power/supply/power_supply_sysfs.c | 16 ++ include/linux/power/generic-adc-battery.h | 23 -- include/linux/power_supply.h | 31 +++ 6 files changed, 301 insertions(+), 210 deletions(-) create mode 100644 Documentation/devicetree/bindings/power/supply/adc-battery.yaml delete mode 100644 include/linux/power/generic-adc-battery.h
Signature · 1 line
2.39.2
Sebastian Reichel
maintainerPATCH 1/11 dt-bindings: power: supply: adc-battery: add binding
Add binding for a battery that is only monitored via ADC channels and simple status GPIOs.
--- .../bindings/power/supply/adc-battery.yaml | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/supply/adc-battery.yaml
Documentation/devicetree/bindings/power/supply/adc-battery.yaml new file +67 -0
@@ -0,0 +1,67 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/power/supply/adc-battery.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: ADC battery + +maintainers: + - Sebastian Reichel <[email protected]> + +description: | + Basic Battery, which only reports (in circuit) voltage and optionally + current via an ADC channel. + +allOf: + - $ref: power-supply.yaml# + +properties: + compatible: + const: adc-battery + + charged-gpios: + description: + GPIO which signals that the battery is fully charged. + maxItems: 1 + + io-channels: + minItems: 1 + maxItems: 3 + + io-channel-names: + oneOf: + - const: voltage + - items: + - const: voltage + - enum: + - current + - power + - items: + - const: voltage + - const: current + - const: power + + monitored-battery: true + +required: + - compatible + - io-channels + - io-channel-names + - monitored-battery + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + fuel-gauge { + compatible = "adc-battery"; + charged-gpios = <&gpio 42 GPIO_ACTIVE_HIGH>; + io-channels = <&adc 13>, <&adc 37>; + io-channel-names = "voltage", "current"; + + power-supplies = <&charger>; + monitored-battery = <&battery>; + };
Signature · 1 line
2.39.2
Re: [PATCHv1 01/11] dt-bindings: power: supply: adc-battery: add binding
Hi Sebastian,
thanks for your patches!
Quoted 4 lines · Sebastian Reichel
Add binding for a battery that is only monitored via ADC channels and simple status GPIOs.
Signed-off-by Sebastian Reichel <[email protected]>
This does look very useful.
Documentation/devicetree/bindings/power/supply/adc-battery.yaml line 14
Quoted 8 lines
Documentation/devicetree/bindings/power/supply/adc-battery.yaml new file +8
-0+title: ADC battery + +maintainers: + - Sebastian Reichel <[email protected]> + +description: | + Basic Battery, which only reports (in circuit) voltage and optionally + current via an ADC channel.
I would over-specify: "voltage over the terminals" and "current out of the battery" so this cannot be misunderstood.
+ this text:
It can also optionally indicate that the battery is full by pulling a GPIO line.
Documentation/devicetree/bindings/power/supply/adc-battery.yaml line 25
Quoted 3 lines
Documentation/devicetree/bindings/power/supply/adc-battery.yaml new file +3
-0+ charged-gpios: + description: + GPIO which signals that the battery is fully charged.
It doesn't say how, I guess either this is an analog circuit (!) or a charger IC? If it doesn't matter, no big deal, but if something is implicit here, then spell it out please.
Quoted 1 line
+ fuel-gauge {
This techno-lingo/slang term is a bit unfortunate, but if there are precedents then stick with it.
The correct term could be something like battery-capacity-meter I suppose.
Yours,
Linus Walleij
Sebastian Reichel
maintainerRe: [PATCHv1 01/11] dt-bindings: power: supply: adc-battery: add binding
Hi Linus,
Quoted 4 lines · Linus Walleij
Hi Sebastian,
thanks for your patches!
Quoted 8 lines
Quoted 4 lines · Sebastian Reichel
Add binding for a battery that is only monitored via ADC channels and simple status GPIOs.
Signed-off-by Sebastian Reichel <[email protected]>This does look very useful.
:)
Quoted 12 lines
Documentation/devicetree/bindings/power/supply/adc-battery.yaml line 14
Quoted 8 lines
Documentation/devicetree/bindings/power/supply/adc-battery.yaml new file +8
-0+title: ADC battery + +maintainers: + - Sebastian Reichel <[email protected]> + +description: | + Basic Battery, which only reports (in circuit) voltage and optionally + current via an ADC channel.I would over-specify: "voltage over the terminals" and "current out of the battery" so this cannot be misunderstood.
Quoted 4 lines
+ this text:
It can also optionally indicate that the battery is full by pulling a GPIO line.
Ack.
Quoted 8 lines
Documentation/devicetree/bindings/power/supply/adc-battery.yaml line 25
Quoted 4 lines
Documentation/devicetree/bindings/power/supply/adc-battery.yaml new file +3
-0+ charged-gpios: + description: + GPIO which signals that the battery is fully charged.It doesn't say how, I guess either this is an analog circuit (!) or a charger IC? If it doesn't matter, no big deal, but if something is implicit here, then spell it out please.
In my case the GPIO is provided by a charger chip, that is not software controllable (just reports charge-done & charger-connected via GPIOs). I've seen something similar in a customer device some years ago. I will add a sentence:
The GPIO is often provided by charger ICs, that are not software controllable.
Quoted 7 lines
Quoted 1 line
+ fuel-gauge {
This techno-lingo/slang term is a bit unfortunate, but if there are precedents then stick with it.
The correct term could be something like battery-capacity-meter I suppose.
Right now in DT we have
- specific node name (e.g. chip names) that should be changed :)
- smart-battery
- battery
- fuel-gauge
I think fuel-gauge is the most sensible of that list, considering hardware vendors usually call their chips battery fuel gauge.
-- Sebastian
Re: [PATCH v2] power: supply: da9150: Fix use after free bug in da9150_charger_remove due to race condition
Quoted 34 lines · Sebastian Reichel <[email protected]> 于2023年3月13日周一 06:31写道:
Hi,
Quoted 22 lines · Zheng Wang
In
da9150_charger_probe,&charger->otg_workis bound withda9150_charger_otg_work.da9150_charger_otg_ncbmay be called to start the work.If we remove the module which will call
da9150_charger_removeto make cleanup, there may be a unfinished work. The possible sequence is as follows:Fix it by canceling the work before cleanup in the
da9150_charger_removeCPU0 CPUc1
|da9150_charger_otg_work
da9150_charger_remove|power_supply_unregister|device_unregister|
power_supply_dev_release|kfree(psy) | | | power_supply_changed(charger->usb); | //useQuoted 7 lines
Fixes c1a281e34dae ("power: Add support for DA9150 Charger")Signed-off-by Zheng Wang <[email protected]>---
v2:
- fix wrong description in commit message and movcancel_work_syncafterusb_unregister_notifiersuggested by Sebastian Reichel
---
Quoted 3 lines
Thanks, queued to power-supply's fixes branch. Please make sure you send your patches to the correct destination next time (linux-scsi should be linux-pm).
Thanks for your effort. I'll keep that in mind :)
Best regards,
Zheng
Quoted 19 lines
Quoted 19 lines
drivers/power/supply/da9150-charger.c | 1 + 1 file changed, 1 insertion(+)drivers/power/supply/da9150-charger.c +1
-1@@ -657,6 +657,7 @@ static int da9150_charger_remove(struct platform_device *pdev)if (!IS_ERR_OR_NULL (charger->usb_phy))usb_unregister_notifier (charger->usb_phy, &charger->otg_nb); +cancel_work_sync (&charger->otg_work);power_supply_unregister (charger->battery);power_supply_unregister (charger->usb); --2.25.1
Re: [PATCHv1 04/11] power: supply: generic-adc-battery: fix unit scaling
In reply to Sebastian ReichelQuoted 3 lines · Sebastian Reichel
There is no mainline board using this driver and I think there never was one. I did add a Fixes tag now, but its probably not worth any backporting trouble considering it has no users.
Good point. If a tree falls in the wood an no-one is there to hear it, it doesn't make a sound.
Yours,
Linus Walleij
Re: [PATCHv1 01/11] dt-bindings: power: supply: adc-battery: add binding
In reply to Sebastian ReichelQuoted 4 lines · Sebastian Reichel
Add binding for a battery that is only monitored via ADC channels and simple status GPIOs.
Signed-off-by Sebastian Reichel <[email protected]>
Thank you for your patch. There is something to discuss/improve.
Documentation/devicetree/bindings/power/supply/adc-battery.yaml line 12
Quoted 5 lines
Documentation/devicetree/bindings/power/supply/adc-battery.yaml new file +5
-0+ +maintainers: + - Sebastian Reichel <[email protected]> + +description: |
Don't need '|'.
Quoted 10 lines
Documentation/devicetree/bindings/power/supply/adc-battery.yaml new file +10
-0+ Basic Battery, which only reports (in circuit) voltage and optionally + current via an ADC channel. + +allOf: + - $ref: power-supply.yaml# + +properties: + compatible: + const: adc-battery +
Documentation/devicetree/bindings/power/supply/adc-battery.yaml line 32
Quoted 10 lines
Documentation/devicetree/bindings/power/supply/adc-battery.yaml new file +10
-0+ charged-gpios: + description: + GPIO which signals that the battery is fully charged. + maxItems: 1 + + io-channels: + minItems: 1 + maxItems: 3 + + io-channel-names:
Simpler:
minItems: 1
items:
- const: voltage
- enum: [ current, power ]
- const: power
Documentation/devicetree/bindings/power/supply/adc-battery.yaml line 44
Quoted 12 lines
Documentation/devicetree/bindings/power/supply/adc-battery.yaml new file +12
-0+ oneOf: + - const: voltage + - items: + - const: voltage + - enum: + - current + - power + - items: + - const: voltage + - const: current + - const: power +
What about temperature? For max17040 this was recently proposed and I wonder whether it is desirable.
https://lore.kernel.org/all/[email protected]/
Best regards,
Krzysztof
Re: [PATCHv1 01/11] dt-bindings: power: supply: adc-battery: add binding
In reply to Sebastian ReichelHi Sebastian,
Quoted 48 lines · Sebastian Reichel
Add binding for a battery that is only monitored via ADC channels and simple status GPIOs.
Signed-off-by Sebastian Reichel <[email protected]>--- .../bindings/power/supply/adc-battery.yaml | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+)create mode 100644
Documentation/devicetree/bindings/power/supply/adc-battery.yamlDocumentation/devicetree/bindings/power/supply/adc-battery.yaml new file +33
-0@@ -0,0 +1,67 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/power/supply/adc-battery.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: ADC battery + +maintainers: + - Sebastian Reichel <[email protected]> + +description: | + Basic Battery, which only reports (in circuit) voltage and optionally + current via an ADC channel. + +allOf: + - $ref: power-supply.yaml# + +properties: + compatible: + const: adc-battery + + charged-gpios: + description: + GPIO which signals that the battery is fully charged. + maxItems: 1 + + io-channels: + minItems: 1 + maxItems: 3 + + io-channel-names: + oneOf:
Documentation/devicetree/bindings/power/supply/adc-battery.yaml line 43
Quoted 10 lines
Documentation/devicetree/bindings/power/supply/adc-battery.yaml new file +10
-0+ - const: voltage + - items: + - const: voltage + - enum: + - current + - power + - items: + - const: voltage + - const: current + - const: power
Good side of not knowing things is being able to asking for more information ;)
So, just by judging these bindings, we have a battery which provides fuel-gauge information via analog line connected to ADC(?)
Reading the description you have here and comments by Linus allows me to assume the line can represent current flowing out of the battery, or the battery voltage.
My guess then is that the io-channel-names property is going to tell which if these properties is being informed by the specific lines, right(?). Do you think you could add some small description for io-channel-names if you respin the series? I'd like to be more certain I "guessed" things right. ;) Maybe also add the 'power' option in the main description which currently just states voltage and power. (Assuming some devices do actually "expose" power levels via these "channels"?
Quoted 24 lines
Documentation/devicetree/bindings/power/supply/adc-battery.yaml new file +24
-0+ + monitored-battery: true + +required: + - compatible + - io-channels + - io-channel-names + - monitored-battery + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + fuel-gauge { + compatible = "adc-battery"; + charged-gpios = <&gpio 42 GPIO_ACTIVE_HIGH>; + io-channels = <&adc 13>, <&adc 37>; + io-channel-names = "voltage", "current"; + + power-supplies = <&charger>; + monitored-battery = <&battery>; + };
Signature · 5 lines
Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~
Sebastian Reichel
maintainerPATCH 2/11 power: supply: core: auto-exposure of simple-battery data
In reply to Sebastian ReichelAdd support for automatically exposing data from the simple-battery firmware node with a single configuration option in the power-supply device.
--- drivers/power/supply/power_supply_core.c | 153 +++++++++++++++++++--- drivers/power/supply/power_supply_sysfs.c | 16 +++ include/linux/power_supply.h | 31 +++++ 3 files changed, 181 insertions(+), 19 deletions(-)
drivers/power/supply/power_supply_core.c +134 -19
@@ -388,7 +388,7 @@ static int __power_supply_get_supplier_property(struct device *dev, void *_data)struct psy_get_supplier_prop_data *data = _data;if (__power_supply_is_supplied_by (epsy, data->psy)) -if (!epsy->desc->get_property (epsy, data->psp, data->val)) +if (!power_supply_get_property (epsy, data->psp, data->val))return 1 ;/* Success */ return 0 ;/* Continue iterating */
@@ -832,6 +832,111 @@ void power_supply_put_battery_info(struct power_supply *psy, }EXPORT_SYMBOL_GPL (power_supply_put_battery_info); +bool power_supply_battery_info_has_prop (struct power_supply_battery_info *info, +enum power_supply_property psp) +{ +if (!info) +return false; + +switch (psp) { +case POWER_SUPPLY_PROP_TECHNOLOGY : +return info->technology !=POWER_SUPPLY_TECHNOLOGY_UNKNOWN ; +case POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN : +return info->energy_full_design_uwh >=0 ; +case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN : +return info->charge_full_design_uah >=0 ; +case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN : +return info->voltage_min_design_uv >=0 ; +case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN : +return info->voltage_max_design_uv >=0 ; +case POWER_SUPPLY_PROP_PRECHARGE_CURRENT : +return info->precharge_current_ua >=0 ; +case POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT : +return info->charge_term_current_ua >=0 ; +case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX : +return info->constant_charge_current_max_ua >=0 ; +case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX : +return info->constant_charge_voltage_max_uv >=0 ; +case POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MIN : +return info->temp_ambient_alert_min >INT_MIN ; +case POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MAX : +return info->temp_ambient_alert_max <INT_MAX ; +case POWER_SUPPLY_PROP_TEMP_ALERT_MIN : +return info->temp_alert_min >INT_MIN ; +case POWER_SUPPLY_PROP_TEMP_ALERT_MAX : +return info->temp_alert_max <INT_MAX ; +case POWER_SUPPLY_PROP_TEMP_MIN : +return info->temp_min >INT_MIN ; +case POWER_SUPPLY_PROP_TEMP_MAX : +return info->temp_max <INT_MAX ; +default : +return false; + } +} +EXPORT_SYMBOL_GPL (power_supply_battery_info_has_prop); + +int power_supply_battery_info_get_prop (struct power_supply_battery_info *info, +enum power_supply_property psp, +union power_supply_propval *val) +{ +if (!info) +return -EINVAL ; + +if (!power_supply_battery_info_has_prop (info, psp)) +return -EINVAL ; + +switch (psp) { +case POWER_SUPPLY_PROP_TECHNOLOGY : + val->intval = info->technology; +return 0 ; +case POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN : + val->intval = info->energy_full_design_uwh; +return 0 ; +case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN : + val->intval = info->charge_full_design_uah; +return 0 ; +case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN : + val->intval = info->voltage_min_design_uv; +return 0 ; +case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN : + val->intval = info->voltage_max_design_uv; +return 0 ; +case POWER_SUPPLY_PROP_PRECHARGE_CURRENT : + val->intval = info->precharge_current_ua; +return 0 ; +case POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT : + val->intval = info->charge_term_current_ua; +return 0 ; +case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX : + val->intval = info->constant_charge_current_max_ua; +return 0 ; +case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX : + val->intval = info->constant_charge_voltage_max_uv; +return 0 ; +case POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MIN : + val->intval = info->temp_ambient_alert_min; +return 0 ; +case POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MAX : + val->intval = info->temp_ambient_alert_max; +return 0 ; +case POWER_SUPPLY_PROP_TEMP_ALERT_MIN : + val->intval = info->temp_alert_min; +return 0 ; +case POWER_SUPPLY_PROP_TEMP_ALERT_MAX : + val->intval = info->temp_alert_max; +return 0 ; +case POWER_SUPPLY_PROP_TEMP_MIN : + val->intval = info->temp_min; +return 0 ; +case POWER_SUPPLY_PROP_TEMP_MAX : + val->intval = info->temp_max; +return 0 ; +default : +return -EINVAL ; + } +} +EXPORT_SYMBOL_GPL (power_supply_battery_info_get_prop); +/** * power_supply_temp2resist_simple() - find the battery internal resistance * percent from temperature
@@ -1046,6 +1151,22 @@ bool power_supply_battery_bti_in_range(struct power_supply_battery_info *info, }EXPORT_SYMBOL_GPL (power_supply_battery_bti_in_range); +static bool psy_has_property (const struct power_supply_desc *psy_desc, +enum power_supply_property psp) +{ +bool found = false; +int i; + +for (i =0 ; i < psy_desc->num_properties; i++) { +if (psy_desc->properties[i] == psp) { + found = true; +break ; + } + } + +return found; +} +int power_supply_get_property (struct power_supply *psy,enum power_supply_property psp,union power_supply_propval *val)
@@ -1056,9 +1177,13 @@ int power_supply_get_property(struct power_supply *psy,return -ENODEV ; } -return psy->desc->get_property (psy, psp, val); +if (psy_has_property (psy->desc, psp)) +return psy->desc->get_property (psy, psp, val); +else if (psy->desc->expose_battery_info) +return power_supply_battery_info_get_prop (psy->battery_info, psp, val); +else +return -EINVAL ; } -EXPORT_SYMBOL_GPL (power_supply_get_property); int power_supply_set_property(struct power_supply *psy, enum power_supply_property psp,
@@ -1117,22 +1242,6 @@ void power_supply_unreg_notifier(struct notifier_block *nb) }EXPORT_SYMBOL_GPL (power_supply_unreg_notifier); -static bool psy_has_property (const struct power_supply_desc *psy_desc, -enum power_supply_property psp) -{ -bool found = false; -int i; - -for (i =0 ; i < psy_desc->num_properties; i++) { -if (psy_desc->properties[i] == psp) { - found = true; -break ; - } - } - -return found; -} -#ifdef CONFIG_THERMAL static int power_supply_read_temp (struct thermal_zone_device *tzd,int *temp)
@@ -1255,6 +1364,12 @@ __power_supply_register(struct device *parent, goto check_supplies_failed; } +if (psy->desc->expose_battery_info) { + rc =power_supply_get_battery_info (psy, &psy->battery_info); +if (rc) + goto check_supplies_failed; + } +spin_lock_init (&psy->changed_lock); rc =device_add (dev);if (rc)
drivers/power/supply/power_supply_sysfs.c +16 -0
@@ -380,6 +380,11 @@ static umode_t power_supply_attr_is_visible(struct kobject *kobj, } } +if (psy->desc->expose_battery_info) { +if (power_supply_battery_info_has_prop (psy->battery_info, attrno)) +return mode; + } +return 0 ; }
@@ -488,6 +493,17 @@ int power_supply_uevent(const struct device *dev, struct kobj_uevent_env *env) goto out; } +if (psy->desc->expose_battery_info) { +for (j =0 ; j <ARRAY_SIZE (power_supply_battery_info_properties); j++) { +if (!power_supply_battery_info_has_prop (psy->battery_info, power_supply_battery_info_properties[j])) +continue ; + ret =add_prop_uevent (dev, env, power_supply_battery_info_properties[j], + prop_buf); +if (ret) + goto out; + } + } + out:free_page ((unsigned long )prop_buf);
include/linux/power_supply.h +31 -0
@@ -275,6 +275,13 @@ struct power_supply_desc {* sensors or other supplies. */ bool no_thermal; +/* +* Set if constant battery information from firmware should be +* exposed automatically. No driver specific code is required +* in that case. If the driver also handles a property provided +* by constant firmware data, the driver's handler is preferred. +*/ +bool expose_battery_info;/* For APM emulation, think legacy userspace. */ int use_for_apm; };
@@ -301,6 +308,7 @@ struct power_supply {bool initialized;bool removing;atomic_t use_cnt; +struct power_supply_battery_info *battery_info;#ifdef CONFIG_THERMAL struct thermal_zone_device *tzd;struct thermal_cooling_device *tcd;
@@ -766,6 +774,24 @@ struct power_supply_battery_info {int bti_resistance_tolerance; }; +static const enum power_supply_property power_supply_battery_info_properties[] = { +POWER_SUPPLY_PROP_TECHNOLOGY , +POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN , +POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN , +POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN , +POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN , +POWER_SUPPLY_PROP_PRECHARGE_CURRENT , +POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT , +POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX , +POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX , +POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MIN , +POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MAX , +POWER_SUPPLY_PROP_TEMP_ALERT_MIN , +POWER_SUPPLY_PROP_TEMP_ALERT_MAX , +POWER_SUPPLY_PROP_TEMP_MIN , +POWER_SUPPLY_PROP_TEMP_MAX , +}; +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);
@@ -795,6 +821,11 @@ extern int power_supply_get_battery_info(struct power_supply *psy, struct power_supply_battery_info **info_out);extern void power_supply_put_battery_info (struct power_supply *psy,struct power_supply_battery_info *info); +extern bool power_supply_battery_info_has_prop (struct power_supply_battery_info *info, +enum power_supply_property psp); +extern int power_supply_battery_info_get_prop (struct power_supply_battery_info *info, +enum power_supply_property psp, +union power_supply_propval *val);extern int power_supply_ocv2cap_simple (struct power_supply_battery_ocv_table *table,int table_len,int ocv); extern struct power_supply_battery_ocv_table *
Signature · 1 line
2.39.2
kernel test robot
build botRe: [PATCHv1 02/11] power: supply: core: auto-exposure of simple-battery data
I love your patch! Perhaps something to improve:
Hi Sebastian,
I love your patch! Perhaps something to improve:
[auto build test WARNING on sre-power-supply/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Sebastian-Reichel/dt-bindings-power-supply-adc-battery-add-binding/20230310-065229 base: https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next patch link: https://lore.kernel.org/r/20230309225041.477440-3-sre%40kernel.org patch subject: [PATCHv1 02/11] power: supply: core: auto-exposure of simple-battery data config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20230310/[email protected]/config) compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/83ec4c841d68a66bc95f5e534a805e765785f934
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Sebastian-Reichel/dt-bindings-power-supply-adc-battery-add-binding/20230310-065229 git checkout 83ec4c841d68a66bc95f5e534a805e765785f934
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 olddefconfig
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/power/supply/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
In file included from drivers/power/supply/power_supply_core.c:21:
>> include/linux/power_supply.h:777:41: warning: 'power_supply_battery_info_properties' defined but not used [-Wunused-const-variable=]
777 | static const enum power_supply_property power_supply_battery_info_properties[] = {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~vim +/power_supply_battery_info_properties +777 include/linux/power_supply.h
776
> 777 static const enum power_supply_property power_supply_battery_info_properties[] = { 778 POWER_SUPPLY_PROP_TECHNOLOGY,
779 POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN,
780 POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN,
781 POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN,
782 POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN,
783 POWER_SUPPLY_PROP_PRECHARGE_CURRENT,
784 POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT,
785 POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX,
786 POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
787 POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MIN,
788 POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MAX,
789 POWER_SUPPLY_PROP_TEMP_ALERT_MIN,
790 POWER_SUPPLY_PROP_TEMP_ALERT_MAX,
791 POWER_SUPPLY_PROP_TEMP_MIN,
792 POWER_SUPPLY_PROP_TEMP_MAX,
793 };
794
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests
kernel test robot
build botRe: [PATCHv1 02/11] power: supply: core: auto-exposure of simple-battery data
In reply to Sebastian ReichelI love your patch! Yet something to improve:
Hi Sebastian,
I love your patch! Yet something to improve:
[auto build test ERROR on sre-power-supply/for-next]
[also build test ERROR on robh/for-next linus/master v6.3-rc1 next-20230310] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Sebastian-Reichel/dt-bindings-power-supply-adc-battery-add-binding/20230310-065229 base: https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next patch link: https://lore.kernel.org/r/20230309225041.477440-3-sre%40kernel.org patch subject: [PATCHv1 02/11] power: supply: core: auto-exposure of simple-battery data config: x86_64-randconfig-a002 (https://download.01.org/0day-ci/archive/20230310/[email protected]/config) compiler: gcc-11 (Debian 11.3.0-8) 11.3.0 reproduce (this is a W=1 build): # https://github.com/intel-lab-lkp/linux/commit/83ec4c841d68a66bc95f5e534a805e765785f934 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Sebastian-Reichel/dt-bindings-power-supply-adc-battery-add-binding/20230310-065229 git checkout 83ec4c841d68a66bc95f5e534a805e765785f934 # save the config file mkdir build_dir && cp config build_dir/.config make W=1 O=build_dir ARCH=x86_64 olddefconfig make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "power_supply_get_property" [drivers/phy/ti/phy-tusb1210.ko] undefined! >> ERROR: modpost: "power_supply_get_property" [drivers/power/supply/88pm860x_charger.ko] undefined! >> ERROR: modpost: "power_supply_get_property" [drivers/power/supply/charger-manager.ko] undefined!
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests
Re: [PATCHv1 02/11] power: supply: core: auto-exposure of simple-battery data
In reply to Sebastian ReichelHi Sebastian,
thanks for your patch!
include/linux/power_supply.h struct power_supply_desc { · line 284
Quoted 7 lines · Sebastian Reichel
include/linux/power_supply.h +7
-0+/* +* Set if constant battery information from firmware should be +* exposed automatically. No driver specific code is required +* in that case. If the driver also handles a property provided +* by constant firmware data, the driver's handler is preferred. +*/ +bool expose_battery_info;
Playing it safe with opt-in I see! But I would probably invert it and add a hide_battery_info for those that don't wanna expose it. It seems pretty useful to just expose this in general.
However I have no insight in what happens on laptops etc for this so I guess you have your reasons, either way:
include/linux/power_supply.h extern int power_supply_get_battery_info(struct power_supply *psy, · line 828
Quoted 5 lines
include/linux/power_supply.h +5
-0+extern bool power_supply_battery_info_has_prop (struct power_supply_battery_info *info, +enum power_supply_property psp); +extern int power_supply_battery_info_get_prop (struct power_supply_battery_info *info, +enum power_supply_property psp, +union power_supply_propval *val);
I think the build robots complain because you need to add some stubs for the not enabled case.
Yours,
Linus Walleij
Re: [PATCHv1 02/11] power: supply: core: auto-exposure of simple-battery data
In reply to Sebastian ReichelQuoted 122 lines · Sebastian Reichel
Add support for automatically exposing data from the simple-battery firmware node with a single configuration option in the power-supply device.
Signed-off-by Sebastian Reichel <[email protected]>--- drivers/power/supply/power_supply_core.c | 153 +++++++++++++++++++--- drivers/power/supply/power_supply_sysfs.c | 16 +++ include/linux/power_supply.h | 31 +++++ 3 files changed, 181 insertions(+), 19 deletions(-)drivers/power/supply/power_supply_core.c +95
-1@@ -388,7 +388,7 @@ static int __power_supply_get_supplier_property(struct device *dev, void *_data)struct psy_get_supplier_prop_data *data = _data;if (__power_supply_is_supplied_by (epsy, data->psy)) -if (!epsy->desc->get_property (epsy, data->psp, data->val)) +if (!power_supply_get_property (epsy, data->psp, data->val))return 1 ;/* Success */ return 0 ;/* Continue iterating */ @@ -832,6 +832,111 @@ void power_supply_put_battery_info(struct power_supply *psy, }EXPORT_SYMBOL_GPL (power_supply_put_battery_info); +bool power_supply_battery_info_has_prop (struct power_supply_battery_info *info, +enum power_supply_property psp) +{ +if (!info) +return false; + +switch (psp) { +case POWER_SUPPLY_PROP_TECHNOLOGY : +return info->technology !=POWER_SUPPLY_TECHNOLOGY_UNKNOWN ; +case POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN : +return info->energy_full_design_uwh >=0 ; +case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN : +return info->charge_full_design_uah >=0 ; +case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN : +return info->voltage_min_design_uv >=0 ; +case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN : +return info->voltage_max_design_uv >=0 ; +case POWER_SUPPLY_PROP_PRECHARGE_CURRENT : +return info->precharge_current_ua >=0 ; +case POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT : +return info->charge_term_current_ua >=0 ; +case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX : +return info->constant_charge_current_max_ua >=0 ; +case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX : +return info->constant_charge_voltage_max_uv >=0 ; +case POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MIN : +return info->temp_ambient_alert_min >INT_MIN ; +case POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MAX : +return info->temp_ambient_alert_max <INT_MAX ; +case POWER_SUPPLY_PROP_TEMP_ALERT_MIN : +return info->temp_alert_min >INT_MIN ; +case POWER_SUPPLY_PROP_TEMP_ALERT_MAX : +return info->temp_alert_max <INT_MAX ; +case POWER_SUPPLY_PROP_TEMP_MIN : +return info->temp_min >INT_MIN ; +case POWER_SUPPLY_PROP_TEMP_MAX : +return info->temp_max <INT_MAX ; +default : +return false; + } +} +EXPORT_SYMBOL_GPL (power_supply_battery_info_has_prop); + +int power_supply_battery_info_get_prop (struct power_supply_battery_info *info, +enum power_supply_property psp, +union power_supply_propval *val) +{ +if (!info) +return -EINVAL ; + +if (!power_supply_battery_info_has_prop (info, psp)) +return -EINVAL ; + +switch (psp) { +case POWER_SUPPLY_PROP_TECHNOLOGY : + val->intval = info->technology; +return 0 ; +case POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN : + val->intval = info->energy_full_design_uwh; +return 0 ; +case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN : + val->intval = info->charge_full_design_uah; +return 0 ; +case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN : + val->intval = info->voltage_min_design_uv; +return 0 ; +case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN : + val->intval = info->voltage_max_design_uv; +return 0 ; +case POWER_SUPPLY_PROP_PRECHARGE_CURRENT : + val->intval = info->precharge_current_ua; +return 0 ; +case POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT : + val->intval = info->charge_term_current_ua; +return 0 ; +case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX : + val->intval = info->constant_charge_current_max_ua; +return 0 ; +case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX : + val->intval = info->constant_charge_voltage_max_uv; +return 0 ; +case POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MIN : + val->intval = info->temp_ambient_alert_min; +return 0 ; +case POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MAX : + val->intval = info->temp_ambient_alert_max; +return 0 ; +case POWER_SUPPLY_PROP_TEMP_ALERT_MIN : + val->intval = info->temp_alert_min; +return 0 ; +case POWER_SUPPLY_PROP_TEMP_ALERT_MAX : + val->intval = info->temp_alert_max; +return 0 ; +case POWER_SUPPLY_PROP_TEMP_MIN :
drivers/power/supply/power_supply_core.c void power_supply_put_battery_info(struct power_supply *psy, · line 938
Quoted 10 lines
drivers/power/supply/power_supply_core.c +10
-0+ val->intval = info->temp_min; +return 0 ; +case POWER_SUPPLY_PROP_TEMP_MAX : + val->intval = info->temp_max; +return 0 ; +default : +return -EINVAL ; + } +} +EXPORT_SYMBOL_GPL (power_supply_battery_info_get_prop);
This is not really relevant for the series. Just speaking it as it came into my mind - I am not expecting changes to this series.
I do very much like the way you have these battery-info APIs not requiring the "struct power_supply *psy". It may be useful for drivers to get the stuff from battery-node prior registering to the power-supply core. I think it'd be nice to have a 'power-supply-info getter API like power_supply_get_battery_info() - which does not require the struct power_supply * but just a device pointer or fwnode pointer. I think it might also be reasonable to pull the battery-info parsing APIs in own file. Or maybe not - definitely up-to you guys. I don't have any active psy-stuff at my hands right now :)
Quoted 96 lines
drivers/power/supply/power_supply_core.c +29
-18+/** * power_supply_temp2resist_simple() - find the battery internal resistance * percent from temperature @@ -1046,6 +1151,22 @@ bool power_supply_battery_bti_in_range(struct power_supply_battery_info *info, }EXPORT_SYMBOL_GPL (power_supply_battery_bti_in_range); +static bool psy_has_property (const struct power_supply_desc *psy_desc, +enum power_supply_property psp) +{ +bool found = false; +int i; + +for (i =0 ; i < psy_desc->num_properties; i++) { +if (psy_desc->properties[i] == psp) { + found = true; +break ; + } + } + +return found; +} +int power_supply_get_property (struct power_supply *psy,enum power_supply_property psp,union power_supply_propval *val)@@ -1056,9 +1177,13 @@ int power_supply_get_property(struct power_supply *psy,return -ENODEV ; } -return psy->desc->get_property (psy, psp, val); +if (psy_has_property (psy->desc, psp)) +return psy->desc->get_property (psy, psp, val); +else if (psy->desc->expose_battery_info) +return power_supply_battery_info_get_prop (psy->battery_info, psp, val); +else +return -EINVAL ; } -EXPORT_SYMBOL_GPL (power_supply_get_property); int power_supply_set_property(struct power_supply *psy, enum power_supply_property psp,@@ -1117,22 +1242,6 @@ void power_supply_unreg_notifier(struct notifier_block *nb) }EXPORT_SYMBOL_GPL (power_supply_unreg_notifier); -static bool psy_has_property (const struct power_supply_desc *psy_desc, -enum power_supply_property psp) -{ -bool found = false; -int i; - -for (i =0 ; i < psy_desc->num_properties; i++) { -if (psy_desc->properties[i] == psp) { - found = true; -break ; - } - } - -return found; -} -#ifdef CONFIG_THERMAL static int power_supply_read_temp (struct thermal_zone_device *tzd,int *temp)@@ -1255,6 +1364,12 @@ __power_supply_register(struct device *parent, goto check_supplies_failed; } +if (psy->desc->expose_battery_info) { + rc =power_supply_get_battery_info (psy, &psy->battery_info); +if (rc) + goto check_supplies_failed; + } +spin_lock_init (&psy->changed_lock); rc =device_add (dev);if (rc)drivers/power/supply/power_supply_sysfs.c +5
-0@@ -380,6 +380,11 @@ static umode_t power_supply_attr_is_visible(struct kobject *kobj, } } +if (psy->desc->expose_battery_info) { +if (power_supply_battery_info_has_prop (psy->battery_info, attrno)) +return mode; + } +return 0 ; }
drivers/power/supply/power_supply_sysfs.c int power_supply_uevent(const struct device *dev, struct kobj_uevent_env *env) · line 501
Quoted 9 lines
drivers/power/supply/power_supply_sysfs.c +6
-0@@ -488,6 +493,17 @@ int power_supply_uevent(const struct device *dev, struct kobj_uevent_env *env) goto out; } +if (psy->desc->expose_battery_info) { +for (j =0 ; j <ARRAY_SIZE (power_supply_battery_info_properties); j++) { +if (!power_supply_battery_info_has_prop (psy->battery_info, power_supply_battery_info_properties[j])) +continue ; + ret =add_prop_uevent (dev, env, power_supply_battery_info_properties[j], + prop_buf);
Usually I do not spot styling things like this - but for some reason it now caught my attention. If you for some reason respin, then you might want to either do this an oneliner - or split the longer line "power_supply_battery_info_has_prop(..." just above.
With or without that:
Signature · 5 lines
Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~
Sebastian Reichel
maintainerPATCH 3/11 power: supply: generic-adc-battery: convert to managed resources
In reply to Sebastian ReichelConvert driver to use managed resources to simplify driver code.
--- drivers/power/supply/generic-adc-battery.c | 81 ++++++---------------- 1 file changed, 23 insertions(+), 58 deletions(-)
drivers/power/supply/generic-adc-battery.c +23 -58
@@ -23,6 +23,7 @@#include <linux/iio/consumer.h> #include <linux/iio/types.h> #include <linux/power/generic-adc-battery.h> +#include <linux/devm-helpers.h> #define JITTER_DEFAULT 10/* hope 10ms is enough */
@@ -266,14 +267,13 @@ static int gab_probe(struct platform_device *pdev)* copying the static properties and allocating extra memory for holding * the extra configurable properties received from platform data. */ - properties =kcalloc (ARRAY_SIZE (gab_props) + -ARRAY_SIZE (gab_chan_name), -sizeof (*properties), -GFP_KERNEL ); -if (!properties) { - ret = -ENOMEM ; - goto first_mem_fail; - } + properties =devm_kcalloc (&pdev->dev, +ARRAY_SIZE (gab_props) + +ARRAY_SIZE (gab_chan_name), +sizeof (*properties), +GFP_KERNEL ); +if (!properties) +return -ENOMEM ;memcpy (properties, gab_props,sizeof (gab_props));
@@ -282,12 +282,13 @@ static int gab_probe(struct platform_device *pdev)* based on the channel supported by consumer device. */ for (chan =0 ; chan <ARRAY_SIZE (gab_chan_name); chan++) { - adc_bat->channel[chan] =iio_channel_get (&pdev->dev, - gab_chan_name[chan]); + adc_bat->channel[chan] =devm_iio_channel_get (&pdev->dev, gab_chan_name[chan]);if (IS_ERR (adc_bat->channel[chan])) { ret =PTR_ERR (adc_bat->channel[chan]); +if (ret != -ENODEV ) +return dev_err_probe (&pdev->dev, ret,"Failed to get ADC channel %s\n" , gab_chan_name[chan]); adc_bat->channel[chan] =NULL ; - }else { + }else if (adc_bat->channel[chan]) {/* copying properties for supported channels only */ int index2;
@@ -302,10 +303,8 @@ static int gab_probe(struct platform_device *pdev) }/* none of the channels are supported so let's bail out */ -if (!any) { - ret = -ENODEV ; - goto second_mem_fail; - } +if (!any) +return dev_err_probe (&pdev->dev, -ENODEV ,"Failed to get any ADC channel\n" );/* * Total number of properties is equal to static properties
@@ -316,25 +315,24 @@ static int gab_probe(struct platform_device *pdev) psy_desc->properties = properties; psy_desc->num_properties = index; - adc_bat->psy =power_supply_register (&pdev->dev, psy_desc, &psy_cfg); -if (IS_ERR (adc_bat->psy)) { - ret =PTR_ERR (adc_bat->psy); - goto err_reg_fail; - } + adc_bat->psy =devm_power_supply_register (&pdev->dev, psy_desc, &psy_cfg); +if (IS_ERR (adc_bat->psy)) +return dev_err_probe (&pdev->dev,PTR_ERR (adc_bat->psy),"Failed to register power-supply device\n" ); -INIT_DELAYED_WORK (&adc_bat->bat_work, gab_work); + ret =devm_delayed_work_autocancel (&pdev->dev, &adc_bat->bat_work, gab_work); +if (ret) +return dev_err_probe (&pdev->dev, ret,"Failed to register delayed work\n" ); - adc_bat->charge_finished =devm_gpiod_get_optional (&pdev->dev, -"charged" ,GPIOD_IN ); + adc_bat->charge_finished =devm_gpiod_get_optional (&pdev->dev,"charged" ,GPIOD_IN );if (adc_bat->charge_finished) {int irq; irq =gpiod_to_irq (adc_bat->charge_finished); - ret =request_any_context_irq (irq, gab_charged, + ret =devm_request_any_context_irq (&pdev->dev, irq, gab_charged,IRQF_TRIGGER_RISING |IRQF_TRIGGER_FALLING ,"battery charged" , adc_bat);if (ret <0 ) - goto gpio_req_fail; +return dev_err_probe (&pdev->dev, ret,"Failed to register irq\n" ); }platform_set_drvdata (pdev, adc_bat);
@@ -343,38 +341,6 @@ static int gab_probe(struct platform_device *pdev)schedule_delayed_work (&adc_bat->bat_work,msecs_to_jiffies (0 ));return 0 ; - -gpio_req_fail: -power_supply_unregister (adc_bat->psy); -err_reg_fail: -for (chan =0 ; chan <ARRAY_SIZE (gab_chan_name); chan++) { -if (adc_bat->channel[chan]) -iio_channel_release (adc_bat->channel[chan]); - } -second_mem_fail: -kfree (properties); -first_mem_fail: -return ret; -} - -static int gab_remove (struct platform_device *pdev) -{ -int chan; -struct gab *adc_bat =platform_get_drvdata (pdev); - -power_supply_unregister (adc_bat->psy); - -if (adc_bat->charge_finished) -free_irq (gpiod_to_irq (adc_bat->charge_finished), adc_bat); - -for (chan =0 ; chan <ARRAY_SIZE (gab_chan_name); chan++) { -if (adc_bat->channel[chan]) -iio_channel_release (adc_bat->channel[chan]); - } - -kfree (adc_bat->psy_desc.properties); -cancel_delayed_work_sync (&adc_bat->bat_work); -return 0 ; }static int __maybe_unused gab_suspend (struct device *dev)
@@ -408,7 +374,6 @@ static struct platform_driver gab_driver = { .pm = &gab_pm_ops, }, .probe = gab_probe, - .remove = gab_remove, };module_platform_driver (gab_driver);
Signature · 1 line
2.39.2
Re: [PATCHv1 03/11] power: supply: generic-adc-battery: convert to managed resources
Quoted 3 lines · Sebastian Reichel
Convert driver to use managed resources to simplify driver code.
Signed-off-by Sebastian Reichel <[email protected]>
Excellent
Yours,
Linus Walleij
Re: [PATCHv1 03/11] power: supply: generic-adc-battery: convert to managed resources
In reply to Sebastian ReichelHi Sebastian, All
To me this does look like a great simplification :)
Quoted 3 lines · Sebastian Reichel
Convert driver to use managed resources to simplify driver code.
Signed-off-by Sebastian Reichel <[email protected]>
Signature · 5 lines
Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~
Sebastian Reichel
maintainerPATCH 4/11 power: supply: generic-adc-battery: fix unit scaling
In reply to Sebastian Reichelpower-supply properties are reported in µV, µA and µW. The IIO API provides mV, mA, mW, so the values need to be multiplied by 1000.
--- drivers/power/supply/generic-adc-battery.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/power/supply/generic-adc-battery.c +3 -0
@@ -136,6 +136,9 @@ static int read_channel(struct gab *adc_bat, enum power_supply_property psp, result);if (ret <0 )pr_err ("read channel error\n" ); +else + *result *=1000 ; +return ret; }
Signature · 1 line
2.39.2
Re: [PATCHv1 04/11] power: supply: generic-adc-battery: fix unit scaling
Quoted 5 lines · Sebastian Reichel
power-supply properties are reported in µV, µA and µW. The IIO API provides mV, mA, mW, so the values need to be multiplied by 1000.
Signed-off-by Sebastian Reichel <[email protected]>
This code can not have seen much testing.
Yours,
Linus Walleij
Re: [PATCHv1 04/11] power: supply: generic-adc-battery: fix unit scaling
In reply to Sebastian ReichelQuoted 5 lines · Sebastian Reichel
power-supply properties are reported in µV, µA and µW. The IIO API provides mV, mA, mW, so the values need to be multiplied by 1000.
Signed-off-by Sebastian Reichel <[email protected]>
As Linus wrote, Fixes-tag would be good. With that remark:
Quoted 18 lines
--- drivers/power/supply/generic-adc-battery.c | 3 +++ 1 file changed, 3 insertions(+)drivers/power/supply/generic-adc-battery.c +3
-0@@ -136,6 +136,9 @@ static int read_channel(struct gab *adc_bat, enum power_supply_property psp, result);if (ret <0 )pr_err ("read channel error\n" ); +else + *result *=1000 ; +return ret; }
Signature · 5 lines
Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~
Sebastian Reichel
maintainerPATCH 5/11 power: supply: generic-adc-battery: drop jitter delay support
In reply to Sebastian ReichelDrop support for configuring IRQ jitter delay by using big enough fixed value.
--- drivers/power/supply/generic-adc-battery.c | 13 ++++--------- include/linux/power/generic-adc-battery.h | 3 --- 2 files changed, 4 insertions(+), 12 deletions(-)
drivers/power/supply/generic-adc-battery.c +4 -9
@@ -227,12 +227,10 @@ static void gab_work(struct work_struct *work)static irqreturn_t gab_charged (int irq,void *dev_id) {struct gab *adc_bat = dev_id; -struct gab_platform_data *pdata = adc_bat->pdata; -int delay; - delay = pdata->jitter_delay ? pdata->jitter_delay :JITTER_DEFAULT ;schedule_delayed_work (&adc_bat->bat_work, -msecs_to_jiffies (delay)); +msecs_to_jiffies (JITTER_DEFAULT )); +return IRQ_HANDLED ; }
@@ -358,14 +356,11 @@ static int __maybe_unused gab_suspend(struct device *dev)static int __maybe_unused gab_resume (struct device *dev) {struct gab *adc_bat =dev_get_drvdata (dev); -struct gab_platform_data *pdata = adc_bat->pdata; -int delay; - - delay = pdata->jitter_delay ? pdata->jitter_delay :JITTER_DEFAULT ;/* Schedule timer to check current status */ schedule_delayed_work (&adc_bat->bat_work, -msecs_to_jiffies (delay)); +msecs_to_jiffies (JITTER_DEFAULT )); +return 0 ; }
include/linux/power/generic-adc-battery.h +0 -3
@@ -11,13 +11,10 @@* @battery_info: recommended structure to specify static power supply * parameters * @cal_charge: calculate charge level. -* @jitter_delay: delay required after the interrupt to check battery -* status.Default set is 10ms. */ struct gab_platform_data {struct power_supply_info battery_info;int (*cal_charge)(long value); -int jitter_delay; };#endif /* GENERIC_ADC_BATTERY_H */
Signature · 1 line
2.39.2
Re: [PATCHv1 05/11] power: supply: generic-adc-battery: drop jitter delay support
Quoted 4 lines · Sebastian Reichel
Drop support for configuring IRQ jitter delay by using big enough fixed value.
Signed-off-by Sebastian Reichel <[email protected]>
Yours,
Linus Walleij
Sebastian Reichel
maintainerPATCH 6/11 power: supply: generic-adc-battery: drop charge now support
In reply to Sebastian ReichelDrop CHARGE_NOW support, which requires a platform specific calculation method.
--- drivers/power/supply/generic-adc-battery.c | 4 ---- include/linux/power/generic-adc-battery.h | 2 -- 2 files changed, 6 deletions(-)
drivers/power/supply/generic-adc-battery.c +0 -4
@@ -72,7 +72,6 @@ static const enum power_supply_property gab_props[] = {POWER_SUPPLY_PROP_STATUS ,POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN ,POWER_SUPPLY_PROP_CHARGE_EMPTY_DESIGN , -POWER_SUPPLY_PROP_CHARGE_NOW ,POWER_SUPPLY_PROP_VOLTAGE_NOW ,POWER_SUPPLY_PROP_CURRENT_NOW , POWER_SUPPLY_PROP_TECHNOLOGY,
@@ -166,9 +165,6 @@ static int gab_get_property(struct power_supply *psy,case POWER_SUPPLY_PROP_CHARGE_EMPTY_DESIGN : val->intval =0 ;break ; -case POWER_SUPPLY_PROP_CHARGE_NOW : - val->intval = pdata->cal_charge (result); -break ;case POWER_SUPPLY_PROP_VOLTAGE_NOW :case POWER_SUPPLY_PROP_CURRENT_NOW :case POWER_SUPPLY_PROP_POWER_NOW :
include/linux/power/generic-adc-battery.h +0 -2
@@ -10,11 +10,9 @@* struct gab_platform_data - platform_data for generic adc iio battery driver. * @battery_info: recommended structure to specify static power supply * parameters -* @cal_charge: calculate charge level. */ struct gab_platform_data {struct power_supply_info battery_info; -int (*cal_charge)(long value); };#endif /* GENERIC_ADC_BATTERY_H */
Signature · 1 line
2.39.2
Re: [PATCHv1 06/11] power: supply: generic-adc-battery: drop charge now support
Quoted 4 lines · Sebastian Reichel
Drop
CHARGE_NOWsupport, which requires a platform specific calculation method.Signed-off-by Sebastian Reichel <[email protected]>
I agree. If we want to support this, we should use the generic methods with interpolation tables defined in DT as well, and it also ideally requires load compensated resistance calculation to figure out Ri so this can bring any kind of reasonable precision.
Yours,
Linus Walleij
Matti Vaittinen
Reviewed-byRe: [PATCHv1 06/11] power: supply: generic-adc-battery: drop charge now support
Hi dee Ho all,
Quoted 11 lines · Linus Walleij
Quoted 4 lines · Sebastian Reichel
Drop
CHARGE_NOWsupport, which requires a platform specific calculation method.Signed-off-by Sebastian Reichel <[email protected]>I agree. If we want to support this, we should use the generic methods with interpolation tables defined in DT as well, and it also ideally requires load compensated resistance calculation to figure out Ri so this can bring any kind of reasonable precision.
I guess you have your reasons, besides you have far better insight to things than I do - hence I am not really objecting this - just asking a question ;)
Do we have generic facilities of computing this based on the DT tables / Ri in place(?) I guess that we do need/see platform specific implementations as long as there is no generic "de-facto" way of doing this available...
Well, maybe this helps kicking things to that direction :)
-- Matti
Signature · 5 lines
Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~
Re: [PATCHv1 06/11] power: supply: generic-adc-battery: drop charge now support
Quoted 13 lines · Matti Vaittinen
Quoted 11 lines · Linus Walleij
Quoted 4 lines · Sebastian Reichel
Drop
CHARGE_NOWsupport, which requires a platform specific calculation method.Signed-off-by Sebastian Reichel <[email protected]>I agree. If we want to support this, we should use the generic methods with interpolation tables defined in DT as well, and it also ideally requires load compensated resistance calculation to figure out Ri so this can bring any kind of reasonable precision.
Quoted 6 lines
I guess you have your reasons, besides you have far better insight to things than I do - hence I am not really objecting this - just asking a question ;)
Do we have generic facilities of computing this based on the DT tables / Ri in place(?)
Not yet, for the Samsung batteries I used a static look-up table derived from the compatible string for calculating Ri from VBAT and from that calculate the capacity from estimated open circuit voltage, see drivers/power/supply/samsung-sdi-battery.c
Quoted 3 lines
I guess that we do need/see platform specific implementations as long as there is no generic "de-facto" way of doing this available...
The method I used with Samsung batteries is fine as long as all you need to know to know everything about a battery is the compatible string. Pretty much any Lion battery with a clearly defined product name can be done this way.
The only reason to put the interpolation tables into the device tree would be to support any random battery, such as one that you do not know the model or this can change.
I am however mildly sceptic about adding that: if you know the VBAT-to-Ri and OCV-to-capacity tables, you must have a datasheet, and then you know the name of the battery product and hence you know the right compatible string...
I think the right way to handle any capacity curves for any battery would be to create static data like I did for the Samsung batteries.
Yours,
Linus Walleij
Sebastian Reichel
maintainerPATCH 7/11 power: supply: generic-adc-battery: drop memory alloc error message
In reply to Sebastian ReichelError printing happens automatically for memory allocation problems.
--- drivers/power/supply/generic-adc-battery.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/power/supply/generic-adc-battery.c +1 -3
@@ -243,10 +243,8 @@ static int gab_probe(struct platform_device *pdev)bool any = false; adc_bat =devm_kzalloc (&pdev->dev,sizeof (*adc_bat),GFP_KERNEL ); -if (!adc_bat) { -dev_err (&pdev->dev,"failed to allocate memory\n" ); +if (!adc_bat)return -ENOMEM ; - } psy_cfg.drv_data = adc_bat; psy_desc = &adc_bat->psy_desc;
Signature · 1 line
2.39.2
Re: [PATCHv1 07/11] power: supply: generic-adc-battery: drop memory alloc error message
Quoted 3 lines · Sebastian Reichel
Error printing happens automatically for memory allocation problems.
Signed-off-by Sebastian Reichel <[email protected]>
Yours,
Linus Walleij
Re: [PATCHv1 07/11] power: supply: generic-adc-battery: drop memory alloc error message
In reply to Sebastian ReichelQuoted 3 lines · Sebastian Reichel
Error printing happens automatically for memory allocation problems.
Signed-off-by Sebastian Reichel <[email protected]>
Quoted 20 lines
--- drivers/power/supply/generic-adc-battery.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)drivers/power/supply/generic-adc-battery.c +1
-3@@ -243,10 +243,8 @@ static int gab_probe(struct platform_device *pdev)bool any = false; adc_bat =devm_kzalloc (&pdev->dev,sizeof (*adc_bat),GFP_KERNEL ); -if (!adc_bat) { -dev_err (&pdev->dev,"failed to allocate memory\n" ); +if (!adc_bat)return -ENOMEM ; - } psy_cfg.drv_data = adc_bat; psy_desc = &adc_bat->psy_desc;
Signature · 5 lines
Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~
Sebastian Reichel
maintainerPATCH 8/11 power: supply: generic-adc-battery: use simple-battery API
In reply to Sebastian ReichelUse standard simple-battery API for constant battery information like min and max voltage. This simplifies the driver a lot and brings automatic support for DT.
--- drivers/power/supply/generic-adc-battery.c | 65 ++-------------------- include/linux/power/generic-adc-battery.h | 18 ------ 2 files changed, 5 insertions(+), 78 deletions(-) delete mode 100644 include/linux/power/generic-adc-battery.h
drivers/power/supply/generic-adc-battery.c +5 -60
@@ -22,7 +22,6 @@#include <linux/slab.h> #include <linux/iio/consumer.h> #include <linux/iio/types.h> -#include <linux/power/generic-adc-battery.h> #include <linux/devm-helpers.h> #define JITTER_DEFAULT 10/* hope 10ms is enough */
@@ -48,9 +47,7 @@ struct gab {struct power_supply *psy;struct power_supply_desc psy_desc;struct iio_channel *channel[GAB_MAX_CHAN_TYPE ]; -struct gab_platform_data *pdata;struct delayed_work bat_work; -int level;int status;bool cable_plugged;struct gpio_desc *charge_finished;
@@ -70,14 +67,6 @@ static void gab_ext_power_changed(struct power_supply *psy)static const enum power_supply_property gab_props[] = {POWER_SUPPLY_PROP_STATUS , -POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN , -POWER_SUPPLY_PROP_CHARGE_EMPTY_DESIGN , -POWER_SUPPLY_PROP_VOLTAGE_NOW , -POWER_SUPPLY_PROP_CURRENT_NOW , -POWER_SUPPLY_PROP_TECHNOLOGY , -POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN , -POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN , -POWER_SUPPLY_PROP_MODEL_NAME , };/*
@@ -97,17 +86,6 @@ static bool gab_charge_finished(struct gab *adc_bat)return gpiod_get_value (adc_bat->charge_finished); } -static int gab_get_status (struct gab *adc_bat) -{ -struct gab_platform_data *pdata = adc_bat->pdata; -struct power_supply_info *bat_info; - - bat_info = &pdata->battery_info; -if (adc_bat->level == bat_info->charge_full_design) -return POWER_SUPPLY_STATUS_FULL ; -return adc_bat->status; -} -static enum gab_chan_type gab_prop_to_chan (enum power_supply_property psp) {switch (psp) {
@@ -144,27 +122,12 @@ static int read_channel(struct gab *adc_bat, enum power_supply_property psp,static int gab_get_property (struct power_supply *psy,enum power_supply_property psp,union power_supply_propval *val) { -struct gab *adc_bat; -struct gab_platform_data *pdata; -struct power_supply_info *bat_info; -int result =0 ; -int ret =0 ; - - adc_bat =to_generic_bat (psy); -if (!adc_bat) { -dev_err (&psy->dev,"no battery infos ?!\n" ); -return -EINVAL ; - } - pdata = adc_bat->pdata; - bat_info = &pdata->battery_info; +struct gab *adc_bat =to_generic_bat (psy);switch (psp) {case POWER_SUPPLY_PROP_STATUS : - val->intval =gab_get_status (adc_bat); -break ; -case POWER_SUPPLY_PROP_CHARGE_EMPTY_DESIGN : - val->intval =0 ; -break ; + val->intval = adc_bat->status; +return 0 ;case POWER_SUPPLY_PROP_VOLTAGE_NOW :case POWER_SUPPLY_PROP_CURRENT_NOW :case POWER_SUPPLY_PROP_POWER_NOW :
@@ -173,26 +136,9 @@ static int gab_get_property(struct power_supply *psy, goto err; val->intval = result;break ; -case POWER_SUPPLY_PROP_TECHNOLOGY : - val->intval = bat_info->technology; -break ; -case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN : - val->intval = bat_info->voltage_min_design; -break ; -case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN : - val->intval = bat_info->voltage_max_design; -break ; -case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN : - val->intval = bat_info->charge_full_design; -break ; -case POWER_SUPPLY_PROP_MODEL_NAME : - val->strval = bat_info->name; -break ;default :return -EINVAL ; } -err: -return ret; }static void gab_work (struct work_struct *work)
@@ -235,7 +181,6 @@ static int gab_probe(struct platform_device *pdev)struct gab *adc_bat;struct power_supply_desc *psy_desc;struct power_supply_config psy_cfg = {}; -struct gab_platform_data *pdata = pdev->dev.platform_data;enum power_supply_property *properties;int ret =0 ;int chan;
@@ -248,7 +193,7 @@ static int gab_probe(struct platform_device *pdev) psy_cfg.drv_data = adc_bat; psy_desc = &adc_bat->psy_desc; - psy_desc->name = pdata->battery_info.name; + psy_desc->name =dev_name (&pdev->dev);/* bootup default values for the battery */ adc_bat->cable_plugged = false;
@@ -256,7 +201,6 @@ static int gab_probe(struct platform_device *pdev) psy_desc->type =POWER_SUPPLY_TYPE_BATTERY ; psy_desc->get_property = gab_get_property; psy_desc->external_power_changed = gab_ext_power_changed; - adc_bat->pdata = pdata;/* * copying the static properties and allocating extra memory for holding
@@ -309,6 +253,7 @@ static int gab_probe(struct platform_device *pdev)*/ psy_desc->properties = properties; psy_desc->num_properties = index; + psy_desc->expose_battery_info = true; adc_bat->psy =devm_power_supply_register (&pdev->dev, psy_desc, &psy_cfg);if (IS_ERR (adc_bat->psy))
include/linux/power/generic-adc-battery.h deleted +0 -18
@@ -1,18 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ -/* -* Copyright (C) 2012, Anish Kumar <[email protected]> -*/ - -#ifndef GENERIC_ADC_BATTERY_H -#define GENERIC_ADC_BATTERY_H - -/** -* struct gab_platform_data - platform_data for generic adc iio battery driver. -* @battery_info: recommended structure to specify static power supply -* parameters -*/ -struct gab_platform_data { -struct power_supply_info battery_info; -}; - -#endif /* GENERIC_ADC_BATTERY_H */
Signature · 1 line
2.39.2
Re: [PATCHv1 08/11] power: supply: generic-adc-battery: use simple-battery API
Quoted 5 lines · Sebastian Reichel
Use standard simple-battery API for constant battery information like min and max voltage. This simplifies the driver a lot and brings automatic support for DT.
Signed-off-by Sebastian Reichel <[email protected]>
That's neat.
Yours,
Linus Walleij
Sebastian Reichel
maintainerPATCH 9/11 power: supply: generic-adc-battery: simplify read_channel logic
In reply to Sebastian ReichelDrop mostly useless gab_prop_to_chan() function by directly supplying the correct enum value to read_channel().
--- drivers/power/supply/generic-adc-battery.c | 31 ++++------------------ 1 file changed, 5 insertions(+), 26 deletions(-)
drivers/power/supply/generic-adc-battery.c +5 -26
@@ -86,31 +86,12 @@ static bool gab_charge_finished(struct gab *adc_bat)return gpiod_get_value (adc_bat->charge_finished); } -static enum gab_chan_type gab_prop_to_chan (enum power_supply_property psp) -{ -switch (psp) { -case POWER_SUPPLY_PROP_POWER_NOW : -return GAB_POWER ; -case POWER_SUPPLY_PROP_VOLTAGE_NOW : -return GAB_VOLTAGE ; -case POWER_SUPPLY_PROP_CURRENT_NOW : -return GAB_CURRENT ; -default : -WARN_ON (1 ); -break ; - } -return GAB_POWER ; -} - -static int read_channel (struct gab *adc_bat,enum power_supply_property psp, +static int read_channel (struct gab *adc_bat,enum gab_chan_type channel,int *result) {int ret; -int chan_index; - chan_index =gab_prop_to_chan (psp); - ret =iio_read_channel_processed (adc_bat->channel[chan_index], - result); + ret =iio_read_channel_processed (adc_bat->channel[channel], result);if (ret <0 )pr_err ("read channel error\n" );else
@@ -129,13 +110,11 @@ static int gab_get_property(struct power_supply *psy, val->intval = adc_bat->status;return 0 ;case POWER_SUPPLY_PROP_VOLTAGE_NOW : +return read_channel (adc_bat,GAB_VOLTAGE , &val->intval);case POWER_SUPPLY_PROP_CURRENT_NOW : +return read_channel (adc_bat,GAB_CURRENT , &val->intval);case POWER_SUPPLY_PROP_POWER_NOW : - ret =read_channel (adc_bat, psp, &result); -if (ret <0 ) - goto err; - val->intval = result; -break ; +return read_channel (adc_bat,GAB_POWER , &val->intval);default :return -EINVAL ; }
Signature · 1 line
2.39.2
Re: [PATCHv1 09/11] power: supply: generic-adc-battery: simplify read_channel logic
Quoted 4 lines · Sebastian Reichel
Drop mostly useless
gab_prop_to_chan()function by directly supplying the correct enum value toread_channel().Signed-off-by Sebastian Reichel <[email protected]>
Looks correct.
Yours,
Linus Walleij
Re: [PATCHv1 09/11] power: supply: generic-adc-battery: simplify read_channel logic
In reply to Sebastian ReichelHi,
Looks good to me.
Quoted 4 lines · Sebastian Reichel
Drop mostly useless
gab_prop_to_chan()function by directly supplying the correct enum value toread_channel().Signed-off-by Sebastian Reichel <[email protected]>
Signature · 5 lines
Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~
Sebastian Reichel
maintainerPATCH 10/11 power: supply: generic-adc-battery: add DT support
In reply to Sebastian ReichelThis adds full DT support to the driver. Because of the previous changes just adding a compatible value is enough.
--- drivers/power/supply/generic-adc-battery.c | 9 +++++++++ 1 file changed, 9 insertions(+)
drivers/power/supply/generic-adc-battery.c +9 -0
@@ -22,6 +22,7 @@#include <linux/slab.h> #include <linux/iio/consumer.h> #include <linux/iio/types.h> +#include <linux/of.h> #include <linux/devm-helpers.h> #define JITTER_DEFAULT 10/* hope 10ms is enough */
@@ -170,6 +171,7 @@ static int gab_probe(struct platform_device *pdev)if (!adc_bat)return -ENOMEM ; + psy_cfg.of_node = pdev->dev.of_node; psy_cfg.drv_data = adc_bat; psy_desc = &adc_bat->psy_desc; psy_desc->name =dev_name (&pdev->dev);
@@ -284,10 +286,17 @@ static int __maybe_unused gab_resume(struct device *dev) static SIMPLE_DEV_PM_OPS(gab_pm_ops, gab_suspend, gab_resume); +static const struct of_device_id gab_match[] = { + { .compatible ="adc-battery" }, + { } +}; +MODULE_DEVICE_TABLE (of, gab_match); +static struct platform_driver gab_driver = { .driver = { .name ="generic-adc-battery" , .pm = &gab_pm_ops, + .of_match_table = gab_match, }, .probe = gab_probe, };
Signature · 1 line
2.39.2
Re: [PATCHv1 10/11] power: supply: generic-adc-battery: add DT support
Quoted 4 lines · Sebastian Reichel
This adds full DT support to the driver. Because of the previous changes just adding a compatible value is enough.
Signed-off-by Sebastian Reichel <[email protected]>
Yours,
Linus Walleij
Re: [PATCHv1 10/11] power: supply: generic-adc-battery: add DT support
In reply to Sebastian ReichelQuoted 4 lines · Sebastian Reichel
This adds full DT support to the driver. Because of the previous changes just adding a compatible value is enough.
Signed-off-by Sebastian Reichel <[email protected]>
Best Regards,
-- Matti
Signature · 5 lines
Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~
Sebastian Reichel
maintainerPATCH 11/11 power: supply: generic-adc-battery: update copyright info
In reply to Sebastian Reicheljz4740-battery.c and s3c_adc_battery.c have been removed from the tree and after all of my restructuring the driver is basically no longer based on them.
Thus update the copyright information and switch to SPDX license identifier while being at it.
--- drivers/power/supply/generic-adc-battery.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)
drivers/power/supply/generic-adc-battery.c +3 -8
@@ -1,13 +1,8 @@ +// SPDX-License-Identifier: GPL-2.0 /* -* Generic battery driver code using IIO +* Generic battery driver using IIO * Copyright (C) 2012, Anish Kumar <[email protected]> -* based on jz4740-battery.c -* based on s3c_adc_battery.c -* -* This file is subject to the terms and conditions of the GNU General Public -* License. See the file COPYING in the main directory of this archive for -* more details. -* +* Copyright (c) 2023, Sebastian Reichel <[email protected]> */ #include <linux/interrupt.h> #include <linux/platform_device.h>
Signature · 1 line
2.39.2
Re: [PATCHv1 11/11] power: supply: generic-adc-battery: update copyright info
Quoted 8 lines · Sebastian Reichel
jz4740-battery.cands3c_adc_battery.chave been removed from the tree and after all of my restructuring the driver is basically no longer based on them.Thus update the copyright information and switch to SPDX license identifier while being at it.
Signed-off-by Sebastian Reichel <[email protected]>
Yours,
Linus Walleij
Re: [PATCHv1 11/11] power: supply: generic-adc-battery: update copyright info
In reply to Sebastian ReichelQuoted 8 lines · Sebastian Reichel
jz4740-battery.cands3c_adc_battery.chave been removed from the tree and after all of my restructuring the driver is basically no longer based on them.Thus update the copyright information and switch to SPDX license identifier while being at it.
Signed-off-by Sebastian Reichel <[email protected]>
Quoted 25 lines
--- drivers/power/supply/generic-adc-battery.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)drivers/power/supply/generic-adc-battery.c +3
-8@@ -1,13 +1,8 @@ +// SPDX-License-Identifier: GPL-2.0 /* -* Generic battery driver code using IIO +* Generic battery driver using IIO * Copyright (C) 2012, Anish Kumar <[email protected]> -* based on jz4740-battery.c -* based on s3c_adc_battery.c -* -* This file is subject to the terms and conditions of the GNU General Public -* License. See the file COPYING in the main directory of this archive for -* more details. -* +* Copyright (c) 2023, Sebastian Reichel <[email protected]> */ #include <linux/interrupt.h> #include <linux/platform_device.h>
Signature · 5 lines
Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~
PATCH v2 1/1 power: supply: da9150: Fix use after free bug in da9150_charger_remove due to race condition
In da9150_charger_probe, &charger->otg_work is bound with da9150_charger_otg_work. da9150_charger_otg_ncb may be called to start the work.
If we remove the module which will call da9150_charger_remove to make cleanup, there may be a unfinished work. The possible sequence is as follows:
Fix it by canceling the work before cleanup in the da9150_charger_remove
CPU0 CPUc1
|da9150_charger_otg_work
da9150_charger_remove |power_supply_unregister |device_unregister |
power_supply_dev_release|
kfree(psy) |
|
| power_supply_changed(charger->usb);
| //use---
v2:
- fix wrong description in commit message and mov cancel_work_sync after usb_unregister_notifier suggested by Sebastian Reichel
--- drivers/power/supply/da9150-charger.c | 1 + 1 file changed, 1 insertion(+)
drivers/power/supply/da9150-charger.c +1 -0
@@ -657,6 +657,7 @@ static int da9150_charger_remove(struct platform_device *pdev)if (!IS_ERR_OR_NULL (charger->usb_phy))usb_unregister_notifier (charger->usb_phy, &charger->otg_nb); +cancel_work_sync (&charger->otg_work);power_supply_unregister (charger->battery);power_supply_unregister (charger->usb);
Signature · 1 line
2.25.1