| Message ID | 20190129085531.32364-1-markz@nvidia.com |
|---|---|
| Headers | show |
| Series | Add max77620 charging & low battery support | expand |
On Tue, 29 Jan 2019, Mark Zhang wrote: > This patch set adds support for max77620 backup battery charging and > low battery monitoring. > > Changes in v2: > - Add devicetree binding documentation > > Mark Zhang (4): > mfd: max77620: Add backup battery charger support > mfd: max77620: add documentation for backup battery charging > mfd: max77620: Add low battery monitor support > mfd: max77620: add documentation for low battery monitoring > > .../devicetree/bindings/mfd/max77620.txt | 34 +++++ > drivers/mfd/max77620.c | 137 +++++++++++++++++- All of this needs moving out to the correct subsystem. drivers/power/supply/max77620-battery.c looks right. > 2 files changed, 170 insertions(+), 1 deletion(-)
On 2/7/2019 4:48 PM, Lee Jones wrote: > On Tue, 29 Jan 2019, Mark Zhang wrote: > >> This patch set adds support for max77620 backup battery charging and >> low battery monitoring. >> >> Changes in v2: >> - Add devicetree binding documentation >> >> Mark Zhang (4): >> mfd: max77620: Add backup battery charger support >> mfd: max77620: add documentation for backup battery charging >> mfd: max77620: Add low battery monitor support >> mfd: max77620: add documentation for low battery monitoring >> >> .../devicetree/bindings/mfd/max77620.txt | 34 +++++ >> drivers/mfd/max77620.c | 137 +++++++++++++++++- > > All of this needs moving out to the correct subsystem. > > drivers/power/supply/max77620-battery.c looks right. Actually max77620 is not a power supply device. This patch set adds 2 features: - Backup battery charger. The RTC in max77620 is supplied from a backup battery and consumes 2.0uA when no other power sources are available. So basically this is not a system battery charger, it's just for RTC. - Low battery monitoring. This is for monitoring the system main battery voltage, so max77620 can shutdown or reset the SoC accordingly. But I think this doesn't conform the idea of "power supply" driver as well. Mark > >> 2 files changed, 170 insertions(+), 1 deletion(-) >
On Tue, 12 Feb 2019, Mark Zhang wrote: > On 2/7/2019 4:48 PM, Lee Jones wrote: > > On Tue, 29 Jan 2019, Mark Zhang wrote: > > > >> This patch set adds support for max77620 backup battery charging and > >> low battery monitoring. > >> > >> Changes in v2: > >> - Add devicetree binding documentation > >> > >> Mark Zhang (4): > >> mfd: max77620: Add backup battery charger support > >> mfd: max77620: add documentation for backup battery charging > >> mfd: max77620: Add low battery monitor support > >> mfd: max77620: add documentation for low battery monitoring > >> > >> .../devicetree/bindings/mfd/max77620.txt | 34 +++++ > >> drivers/mfd/max77620.c | 137 +++++++++++++++++- > > > > All of this needs moving out to the correct subsystem. > > > > drivers/power/supply/max77620-battery.c looks right. > > Actually max77620 is not a power supply device. This patch set adds 2 > features: > - Backup battery charger. The RTC in max77620 is supplied from a backup > battery and consumes 2.0uA when no other power sources are available. So > basically this is not a system battery charger, it's just for RTC. > - Low battery monitoring. This is for monitoring the system main battery > voltage, so max77620 can shutdown or reset the SoC accordingly. But I > think this doesn't conform the idea of "power supply" driver as well. Most other battery handling seems to happen in drivers/power/supply/*.battery*. If that's not the right location, then you need to find a place for it to go. MFDs do not provide useful functionality per say - that is the role of the child devices.
On 2/12/2019 4:04 PM, Lee Jones wrote: > On Tue, 12 Feb 2019, Mark Zhang wrote: > >> On 2/7/2019 4:48 PM, Lee Jones wrote: >>> On Tue, 29 Jan 2019, Mark Zhang wrote: >>> >>>> This patch set adds support for max77620 backup battery charging and >>>> low battery monitoring. >>>> >>>> Changes in v2: >>>> - Add devicetree binding documentation >>>> >>>> Mark Zhang (4): >>>> mfd: max77620: Add backup battery charger support >>>> mfd: max77620: add documentation for backup battery charging >>>> mfd: max77620: Add low battery monitor support >>>> mfd: max77620: add documentation for low battery monitoring >>>> >>>> .../devicetree/bindings/mfd/max77620.txt | 34 +++++ >>>> drivers/mfd/max77620.c | 137 +++++++++++++++++- >>> >>> All of this needs moving out to the correct subsystem. >>> >>> drivers/power/supply/max77620-battery.c looks right. >> >> Actually max77620 is not a power supply device. This patch set adds 2 >> features: >> - Backup battery charger. The RTC in max77620 is supplied from a backup >> battery and consumes 2.0uA when no other power sources are available. So >> basically this is not a system battery charger, it's just for RTC. >> - Low battery monitoring. This is for monitoring the system main battery >> voltage, so max77620 can shutdown or reset the SoC accordingly. But I >> think this doesn't conform the idea of "power supply" driver as well. > > Most other battery handling seems to happen in > drivers/power/supply/*.battery*. If that's not the right location, > then you need to find a place for it to go. MFDs do not provide > useful functionality per say - that is the role of the child devices. > Understood. Looking at "enum power_supply_property", battery handling drivers are able to get battery capacity/current voltage/temperature/status/... and etc, which is making sense. But my patch set doesn't do this kind of things at all so I think the power supply framework doesn't fit here. Let me check other similar device drivers... hope to get inspired. Thanks Lee. Mark