| Message ID | cover.1585902279.git.matti.vaittinen@fi.rohmeurope.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Support ROHM BD99954 charger IC | expand |
Hello Mark, Sebastian, All, On Fri, 2020-04-03 at 11:36 +0300, Matti Vaittinen wrote: > Support ROHM BD99954 Battery Management IC > > ROHM BD99954 is a Battery Management IC for 1-4 cell Lithium-Ion > secondary battery. BD99954 is intended to be used in space-constraint > equipment such as Low profile Notebook PC, Tablets and other > applications. > > Series extracts a "linear ranges" helper out of the regulator > framework. Linear ranges helper is intended to help converting > real-world values to register values when conversion is linear. I > suspect this is useful also for power subsystem and possibly for clk. I see Mark has acked/reviewed both the regulator changes and linear_ranges code. Do you think Mark should take the linear_ranges and regulator changes in his tree? I don't know Sebastian's schedule or when the charger portion is good to go - but I know that each new regulator driver which is added to regulator tree has a chance of using the struct regulator_linear_range - which will break when this series is applied. Or what would be the best way to avoid breaking regulators? OTOH, if Mark takes linear_ranges in his tree, then this power portion of the series will depend on linear_ranges stuff that is in regulator tree. I guess this must be pretty standard stuff for you and you probably know how to handle it but I just wanted to point out the risk of breaking regulator build without visible merge conflicts. Please let me know if I should split the series and rebase linear_ranges / regulator stuff on top of regulator tree. Br, Matti Vaittinen
On Fri, Apr 03, 2020 at 10:07:41AM +0000, Vaittinen, Matti wrote: > On Fri, 2020-04-03 at 11:36 +0300, Matti Vaittinen wrote: > > Support ROHM BD99954 Battery Management IC > > > > ROHM BD99954 is a Battery Management IC for 1-4 cell Lithium-Ion > > secondary battery. BD99954 is intended to be used in space-constraint > > equipment such as Low profile Notebook PC, Tablets and other > > applications. > > > > Series extracts a "linear ranges" helper out of the regulator > > framework. Linear ranges helper is intended to help converting > > real-world values to register values when conversion is linear. I > > suspect this is useful also for power subsystem and possibly for clk. > > I see Mark has acked/reviewed both the regulator changes and > linear_ranges code. Do you think Mark should take the linear_ranges and > regulator changes in his tree? I don't know Sebastian's schedule or > when the charger portion is good to go - but I know that each new > regulator driver which is added to regulator tree has a chance of using > the struct regulator_linear_range - which will break when this series > is applied. Or what would be the best way to avoid breaking regulators? > > OTOH, if Mark takes linear_ranges in his tree, then this power portion > of the series will depend on linear_ranges stuff that is in regulator > tree. I guess this must be pretty standard stuff for you and you > probably know how to handle it but I just wanted to point out the risk > of breaking regulator build without visible merge conflicts. > > Please let me know if I should split the series and rebase > linear_ranges / regulator stuff on top of regulator tree. From my point of view, you need to wait till rc1 is out and rebase the series. The cross-subsystem changes can be handled by maintainers in a form of immutable branches / tags. On your side you may recommend them how to proceed, but the final decision is by them.
Hello Andy & All, On Fri, 2020-04-03 at 14:02 +0300, andriy.shevchenko@linux.intel.com wrote: > On Fri, Apr 03, 2020 at 10:07:41AM +0000, Vaittinen, Matti wrote: > > On Fri, 2020-04-03 at 11:36 +0300, Matti Vaittinen wrote: > > > Support ROHM BD99954 Battery Management IC > > > > > > ROHM BD99954 is a Battery Management IC for 1-4 cell Lithium-Ion > > > secondary battery. BD99954 is intended to be used in space- > > > constraint > > > equipment such as Low profile Notebook PC, Tablets and other > > > applications. > > > > > > Series extracts a "linear ranges" helper out of the regulator > > > framework. Linear ranges helper is intended to help converting > > > real-world values to register values when conversion is linear. I > > > suspect this is useful also for power subsystem and possibly for > > > clk. > > > > I see Mark has acked/reviewed both the regulator changes and > > linear_ranges code. Do you think Mark should take the linear_ranges > > and > > regulator changes in his tree? I don't know Sebastian's schedule or > > when the charger portion is good to go - but I know that each new > > regulator driver which is added to regulator tree has a chance of > > using > > the struct regulator_linear_range - which will break when this > > series > > is applied. Or what would be the best way to avoid breaking > > regulators? > > > > OTOH, if Mark takes linear_ranges in his tree, then this power > > portion > > of the series will depend on linear_ranges stuff that is in > > regulator > > tree. I guess this must be pretty standard stuff for you and you > > probably know how to handle it but I just wanted to point out the > > risk > > of breaking regulator build without visible merge conflicts. > > > > Please let me know if I should split the series and rebase > > linear_ranges / regulator stuff on top of regulator tree. > > From my point of view, you need to wait till rc1 is out and rebase > the series. > The cross-subsystem changes can be handled by maintainers in a form > of > immutable branches / tags. On your side you may recommend them how to > proceed, > but the final decision is by them. > Thanks Andy. I re-read what I wrote and I see it can be interpreted as if I was trying to tell how things should be done. That was my intention. My intention was to point out that my patches will break regulator tree builds if new drivers are added. > From my point of view, you need to wait till rc1 is out and rebase > the series. Does this mean that there is no new regulator drivers expected to be added after rc1 is out? If this is the case, the rebasing this series on top of rc1 should work as then I get all new drivers (for a release) converted. This is of course fine by me - but again we will risk of breaking regulators if the series slips to next release. Thus I thought that perhaps we should try getting the regulators stuff in Marks tree so that further reguator drivers wouldn't be broken. But as I said, my intention is not to claim I know how to do this. On the contrary - I have _never_ participated in maintaining a tree that will be merged by others. So, please just let me know what you see the best. I can do splitting the series or rebasing to regulator tree or rebase to rc1 when it is out if required :) Br, Matti
On Fri, Apr 03, 2020 at 11:13:54AM +0000, Vaittinen, Matti wrote: > On Fri, 2020-04-03 at 14:02 +0300, andriy.shevchenko@linux.intel.com > wrote: > > On Fri, Apr 03, 2020 at 10:07:41AM +0000, Vaittinen, Matti wrote: > > > On Fri, 2020-04-03 at 11:36 +0300, Matti Vaittinen wrote: ... > > From my point of view, you need to wait till rc1 is out and rebase > > the series. > > Does this mean that there is no new regulator drivers expected to be > added after rc1 is out? If this is the case, the rebasing this series > on top of rc1 should work as then I get all new drivers (for a release) > converted. This is of course fine by me - but again we will risk of > breaking regulators if the series slips to next release. Thus I thought > that perhaps we should try getting the regulators stuff in Marks tree > so that further reguator drivers wouldn't be broken. We already almost in the middle of merge window. With such a change I believe there is no room for testing it, so, it will be quite unlikely it makes v5.7 release. That is, you have approximately 6 more weeks to polish this code. > But as I said, my intention is not to claim I know how to do this. On > the contrary - I have _never_ participated in maintaining a tree that > will be merged by others. So, please just let me know what you see the > best. I can do splitting the series or rebasing to regulator tree or > rebase to rc1 when it is out if required :)
On Fri, Apr 03, 2020 at 02:50:16PM +0300, andriy.shevchenko@linux.intel.com wrote: > On Fri, Apr 03, 2020 at 11:13:54AM +0000, Vaittinen, Matti wrote: > > On Fri, 2020-04-03 at 14:02 +0300, andriy.shevchenko@linux.intel.com > > wrote: > > > On Fri, Apr 03, 2020 at 10:07:41AM +0000, Vaittinen, Matti wrote: > > > > On Fri, 2020-04-03 at 11:36 +0300, Matti Vaittinen wrote: > > ... > > > > From my point of view, you need to wait till rc1 is out and rebase > > > the series. > > > > Does this mean that there is no new regulator drivers expected to be > > added after rc1 is out? If this is the case, the rebasing this series > > on top of rc1 should work as then I get all new drivers (for a release) > > converted. This is of course fine by me - but again we will risk of > > breaking regulators if the series slips to next release. Thus I thought > > that perhaps we should try getting the regulators stuff in Marks tree > > so that further reguator drivers wouldn't be broken. > > We already almost in the middle of merge window. With such a change I believe > there is no room for testing it, so, it will be quite unlikely it makes v5.7 > release. > > That is, you have approximately 6 more weeks to polish this code. (If you need so, of course) > > But as I said, my intention is not to claim I know how to do this. On > > the contrary - I have _never_ participated in maintaining a tree that > > will be merged by others. So, please just let me know what you see the > > best. I can do splitting the series or rebasing to regulator tree or > > rebase to rc1 when it is out if required :)
On Fri, Apr 03, 2020 at 11:13:54AM +0000, Vaittinen, Matti wrote: > On Fri, 2020-04-03 at 14:02 +0300, andriy.shevchenko@linux.intel.com > > From my point of view, you need to wait till rc1 is out and rebase > > the series. > > The cross-subsystem changes can be handled by maintainers in a form > > of > > immutable branches / tags. On your side you may recommend them how to > > proceed, > > but the final decision is by them. > Thanks Andy. I re-read what I wrote and I see it can be interpreted as > if I was trying to tell how things should be done. That was my > intention. My intention was to point out that my patches will break > regulator tree builds if new drivers are added. > > From my point of view, you need to wait till rc1 is out and rebase > > the series. > Does this mean that there is no new regulator drivers expected to be > added after rc1 is out? If this is the case, the rebasing this series > on top of rc1 should work as then I get all new drivers (for a release) During the merge window no new anything except bug fixes is expected to be applied. Like Andy says we'll share a branch for any dependencies, nobody in particular seems to apply code for lib so I guess I'll take that patch and the regulator one and share it but not until after the merge window.
On Fri, 2020-04-03 at 13:01 +0100, Mark Brown wrote: > On Fri, Apr 03, 2020 at 11:13:54AM +0000, Vaittinen, Matti wrote: > > On Fri, 2020-04-03 at 14:02 +0300, > > andriy.shevchenko@linux.intel.com > > > From my point of view, you need to wait till rc1 is out and > > > rebase > > > the series. > > > The cross-subsystem changes can be handled by maintainers in a > > > form > > > of > > > immutable branches / tags. On your side you may recommend them > > > how to > > > proceed, > > > but the final decision is by them. > > Thanks Andy. I re-read what I wrote and I see it can be interpreted > > as > > if I was trying to tell how things should be done. That was my > > intention. My intention was to point out that my patches will break > > regulator tree builds if new drivers are added. > > > From my point of view, you need to wait till rc1 is out and > > > rebase > > > the series. > > Does this mean that there is no new regulator drivers expected to > > be > > added after rc1 is out? If this is the case, the rebasing this > > series > > on top of rc1 should work as then I get all new drivers (for a > > release) > > During the merge window no new anything except bug fixes is expected > to > be applied. Like Andy says we'll share a branch for any > dependencies, > nobody in particular seems to apply code for lib so I guess I'll take > that patch and the regulator one and share it but not until after the > merge window. Thanks for taking it Mark. So I should rebase and resend when v5.7-rc1 is tagged as Andy suggested? --Matti
On Fri, Apr 03, 2020 at 12:30:17PM +0000, Vaittinen, Matti wrote: > Thanks for taking it Mark. So I should rebase and resend when v5.7-rc1 > is tagged as Andy suggested? Yes.
Hi, On Fri, Apr 03, 2020 at 11:36:17AM +0300, Matti Vaittinen wrote: > Matti Vaittinen (10): > dt-bindings: battery: add new battery parameters > dt_bindings: ROHM BD99954 Charger > lib: add linear ranges helpers > lib/test_linear_ranges: add a test for the 'linear_ranges' > power: supply: bd70528: rename linear_range to avoid collision > regulator: use linear_ranges helper > power: supply: bd70528: use linear ranges > power: supply: add battery parameters > power: supply: Support ROHM bd99954 charger > power: supply: Fix Kconfig help text indentiation Can you please rebase the series, so that it is ordered with the linear ranges and regulator patches in the beginning? That way Mark can take the first three patches through the regulator tree and provide an immutable branch to me without requiring all of the power-supply specific patches. E.g. like this: > lib: add linear ranges helpers > lib/test_linear_ranges: add a test for the 'linear_ranges' > regulator: use linear_ranges helper > dt-bindings: battery: add new battery parameters > dt_bindings: ROHM BD99954 Charger > power: supply: bd70528: rename linear_range to avoid collision > power: supply: bd70528: use linear ranges > power: supply: add battery parameters > power: supply: Support ROHM bd99954 charger > power: supply: Fix Kconfig help text indentiation -- Sebastian
Hello Sebastian, On Sun, 2020-04-05 at 05:27 +0200, Sebastian Reichel wrote: > Hi, > > On Fri, Apr 03, 2020 at 11:36:17AM +0300, Matti Vaittinen wrote: > > Matti Vaittinen (10): > > dt-bindings: battery: add new battery parameters > > dt_bindings: ROHM BD99954 Charger > > lib: add linear ranges helpers > > lib/test_linear_ranges: add a test for the 'linear_ranges' > > power: supply: bd70528: rename linear_range to avoid collision > > regulator: use linear_ranges helper > > power: supply: bd70528: use linear ranges > > power: supply: add battery parameters > > power: supply: Support ROHM bd99954 charger > > power: supply: Fix Kconfig help text indentiation > > Can you please rebase the series, so that it is ordered with the > linear ranges and regulator patches in the beginning? That way > Mark can take the first three patches through the regulator tree > and provide an immutable branch to me without requiring all of > the power-supply specific patches. E.g. like this: I can and will do rebasing on top of 5.7-rc1 anyways :) But the power: supply: bd70528: rename linear_range to avoid collision should probably come before regulator: use linear_ranges helper because the BD70528 is MFD device and regulator headers get included via BD70528 MFD headers. What is the best ordering for the patches? Or would Mark also take the power: supply: bd70528: rename linear_range to avoid collision ? Best Regards Matti Vaittinen
Support ROHM BD99954 Battery Management IC ROHM BD99954 is a Battery Management IC for 1-4 cell Lithium-Ion secondary battery. BD99954 is intended to be used in space-constraint equipment such as Low profile Notebook PC, Tablets and other applications. Series extracts a "linear ranges" helper out of the regulator framework. Linear ranges helper is intended to help converting real-world values to register values when conversion is linear. I suspect this is useful also for power subsystem and possibly for clk. Current implementation does not support inversely proportional ranges but adding support for that could be helpful. This version of series introduces new battry DT binding entries and adds the parsing in power_supply_get_battery_info(). Changelog v8: Linear ranges - small improvements suggested by Andy Shevchenko, no functional changes Changelog v7: General: - rebased on top of v5.6 bd99954 driver: - fixed bunch of styling issues spotted by Andy Shevchenko - dropped ACPI table as ACPI properties are not supported at this version of driver. - added few acks Changelog v6: generic: - rebased on top of 5.6-rc7. linear ranges: - moved to lib as requested by Greg KH - EXPORT_SYMBOL => EXPORT_SYMBOL_GPL - licence GPL-2.0, not later - added KUnit test for linear ranges Changelog v5: generic: - rebased on top of 5.6-rc6. DT-bindings: - Dropped -charger extension from compatible and removed wildcard x. regulators: - squashed the regulator changes in one patch. power-supply KConfig: - fixed indentiation - dropped unnecessary 'default N' from BD99954. Changelog v4: generic: - rebase and drop RFC. DT-bindings: - add I2C node address-cells and size-cells to fix yaml check errors - uncomment multipleOf: bd70528: - add patch which renames driver internal linear_range struct to avoid collision when regulator/driver.h (which gets included from rohm generic header) introduces the linear_range struct. regulators: - rebase to v5.6-rc2 and convert also the two newly introduced drivers to use linear_range struct instead of regulator_linear_range. linear_ranges: - Fix kerneldoc. Changelog RFC-v3: DT-bindings: - fix the BD99954 binding (the *-microvolt Vs. *-microvolts issue is still there. Not sure which one is correct) - renabe tricklecharge-* binding to trickle-charge-* as suggested by Rob. - drop the linear-ranges helper which was written for BD70528 and extract the linear-range code from regulator framework instead. - refactor regulator framework to utilize extracted linear-ranges code. - change the struct regulator_linear_range to linear_range from regulator drivers. - refactor BD70528 to use regulator framework originated linear-ranges code. - change BD99954 to use linear-ranges code from regulator framework Changelog RFC-v2: DT-bindings: - Used the battery parameters described in battery.txt - Added few new parameters to battery.txt - Added ASCII art charging profile chart for BD99954 to explain states and limits. Linear ranges: - Fixed division by zero error from linear-ranges code if step 0 is used. Power-supply core: - Added parsing of new battery parameters. BD99954 driver: - converted to use battery parameters from battery node - Added step 0 ranges for reg values which do not change voltage - added dt-node to psy-config Patch 1: DT binding docs for the new battery parameters Patch 2: BD99954 charger DT binding docs Patch 3: Linear ranges helpers Patch 4: Test for linear ranges helpers Patch 5: Rename driver internal struct linear_range from bd70528-power Patch 6: Use linear-ranges helpers in regulator framework and convert regulator drivers to use new linear_range struct. Patch 7: Use linear-ranges helpers in bd70528 driver Patch 8: Parsing of new battery parameters Patch 9: ROHM BD99954 charger IC driver Patch 10: Fix Kconfig help text indentiation for other entries as well. --- Matti Vaittinen (10): dt-bindings: battery: add new battery parameters dt_bindings: ROHM BD99954 Charger lib: add linear ranges helpers lib/test_linear_ranges: add a test for the 'linear_ranges' power: supply: bd70528: rename linear_range to avoid collision regulator: use linear_ranges helper power: supply: bd70528: use linear ranges power: supply: add battery parameters power: supply: Support ROHM bd99954 charger power: supply: Fix Kconfig help text indentiation .../bindings/power/supply/battery.txt | 6 + .../bindings/power/supply/rohm,bd99954.yaml | 155 +++ drivers/power/supply/Kconfig | 34 +- drivers/power/supply/Makefile | 1 + drivers/power/supply/bd70528-charger.c | 140 +- drivers/power/supply/bd99954-charger.c | 1149 +++++++++++++++++ drivers/power/supply/power_supply_core.c | 8 + drivers/regulator/88pg86x.c | 4 +- drivers/regulator/88pm800-regulator.c | 4 +- drivers/regulator/Kconfig | 1 + drivers/regulator/act8865-regulator.c | 4 +- drivers/regulator/act8945a-regulator.c | 2 +- drivers/regulator/arizona-ldo1.c | 2 +- drivers/regulator/arizona-micsupp.c | 4 +- drivers/regulator/as3711-regulator.c | 6 +- drivers/regulator/as3722-regulator.c | 4 +- drivers/regulator/axp20x-regulator.c | 16 +- drivers/regulator/bcm590xx-regulator.c | 8 +- drivers/regulator/bd70528-regulator.c | 8 +- drivers/regulator/bd71828-regulator.c | 10 +- drivers/regulator/bd718x7-regulator.c | 26 +- drivers/regulator/da903x.c | 2 +- drivers/regulator/helpers.c | 130 +- drivers/regulator/hi6421-regulator.c | 4 +- drivers/regulator/lochnagar-regulator.c | 4 +- drivers/regulator/lp873x-regulator.c | 4 +- drivers/regulator/lp87565-regulator.c | 2 +- drivers/regulator/lp8788-buck.c | 2 +- drivers/regulator/max77650-regulator.c | 2 +- drivers/regulator/mcp16502.c | 4 +- drivers/regulator/mp8859.c | 2 +- drivers/regulator/mt6323-regulator.c | 6 +- drivers/regulator/mt6358-regulator.c | 8 +- drivers/regulator/mt6380-regulator.c | 6 +- drivers/regulator/mt6397-regulator.c | 6 +- drivers/regulator/palmas-regulator.c | 4 +- drivers/regulator/qcom-rpmh-regulator.c | 2 +- drivers/regulator/qcom_rpm-regulator.c | 14 +- drivers/regulator/qcom_smd-regulator.c | 70 +- drivers/regulator/rk808-regulator.c | 10 +- drivers/regulator/s2mps11.c | 14 +- drivers/regulator/sky81452-regulator.c | 2 +- drivers/regulator/stpmic1_regulator.c | 18 +- drivers/regulator/tps65086-regulator.c | 10 +- drivers/regulator/tps65217-regulator.c | 4 +- drivers/regulator/tps65218-regulator.c | 6 +- drivers/regulator/tps65912-regulator.c | 4 +- drivers/regulator/twl-regulator.c | 4 +- drivers/regulator/twl6030-regulator.c | 2 +- drivers/regulator/wm831x-dcdc.c | 2 +- drivers/regulator/wm831x-ldo.c | 4 +- drivers/regulator/wm8350-regulator.c | 2 +- drivers/regulator/wm8400-regulator.c | 2 +- include/linux/linear_range.h | 48 + include/linux/power/bd99954-charger.h | 1075 +++++++++++++++ include/linux/power_supply.h | 4 + include/linux/regulator/driver.h | 27 +- lib/Kconfig | 3 + lib/Kconfig.debug | 11 + lib/Makefile | 2 + lib/linear_ranges.c | 241 ++++ lib/test_linear_ranges.c | 228 ++++ 62 files changed, 3231 insertions(+), 356 deletions(-) create mode 100644 Documentation/devicetree/bindings/power/supply/rohm,bd99954.yaml create mode 100644 drivers/power/supply/bd99954-charger.c create mode 100644 include/linux/linear_range.h create mode 100644 include/linux/power/bd99954-charger.h create mode 100644 lib/linear_ranges.c create mode 100644 lib/test_linear_ranges.c base-commit: 7111951b8d49 ("Linux 5.6")