mfd: Implement devicetree support for AB8500 Btemp
Quoted 5 lines
Date: Tue, 10 Jul 2012 14:12:01 +0000
From: Arnd Bergmann <[email protected]>
To: linux-arm-kernel at lists.infradead.org
Cc: linaro-dev at lists.linaro.org, linux-kernel at vger.kernel.org,
"Rajanikanth H.V" <[email protected]>,
patches at linaro.org
Quoted 31 lines
Subject: Re: [PATCH] mfd: Implement devicetree support for AB8500
Btemp
Message-ID: <[email protected]>
Content-Type: Text/Plain; charset="utf-8"Quoted 24 lines · Rajanikanth H.V
Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt new file +21
-0@@ -0,0 +1,54 @@ +AB8500 Battery Termperature Monitor Driver + +AB8500 is a mixed signal multimedia and power management +device comprising: power and energy management module, +WalliCharger and USB charger interface, audio, general +purpose ADC TVOut, clock management and SIM card Interface. + +Battery temperature monitoring support is part of 'energy +management module', the other components of this module +are: 'main and USB Combo charger' and fuel guage. + +The properties below describes the node for battery +temperature monitor driver. + +Required Properties: +- compatible = "stericsson,ab8500-btemp" + +interrupts: + Four battery temperature ranges are be defined + which results in interrupt events as: + - Btemp
Quoted 11 lines
Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt line 25
Quoted 4 lines
Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt new file +4
-0+ - BtempLow + - BtempMedium + - BtempHigh +These names do not match the five interrupts in the example or in the code. When you provide an "interrupt-names" property you have to define the exact strings that are permissible for them in the binding.
Quoted 2 lines
Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt new file +2
-0+Supplied-to: + This shall be power supply class dependency where in the
runtime battery
Quoted 1 line
Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt line 28
Quoted 1 line
Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt new file +1
-0+ properties will be shared across fuel guage and charging
algorithm driver.
Quoted 2 lines
I probably don't understand enough of this, but shouldn't the other
devices
Quoted 2 lines
that are supplied by this have a reference to this node rather than doing it this way around? Why use strings here instead of phandles?
This is a logical binding w.r.t power supply event change across energy-management-module drivers where in runtime battery properties are shared along with uevent notification.
ref: di->btemp_psy.external_power_changed =
ab8500_btemp_external_power_changed;
ref: ab8500_btemp.cNeed for this property:
btemp, fg and charger updates power-supply properties based on the events listed above.
Event handler invokes power supply change notifier
which in-turn invokes registered power supply class call-back based on the 'supplied_to' string.
ref:
power_supply_changed_work(..) ./drivers/power/power_supply_core.c
In this case how to approach through phandle?
Quoted 5 lines
You are also not listing some of the properties that are in the device tree here, like the "interrupts" property itself.
Review
Quoted 1 line
(quoted diff) +0
-0
b/arch/arm/mach-ux500/board-mop500-bm.c
Quoted 27 lines
arch/arm/mach-ux500/board-mop500-bm.c
Quoted 4 lines
new file mode 100644
index 0000000..3349cebarch/arm/mach-ux500/board-mop500-bm.c new file +0
-0Didn't we conclude that this file has no board-specific information in it? Either explain why it's still here, or move it into the driver itself.
arch/arm/mach-ux500/board-mop500-bm.c
Quoted 9 lines
arch/arm/mach-ux500/board-mop500-bm.c new file +9
-0+/* +* Note that the batres_vs_temp table must be strictly sorted by falling +* temperature values to work. +*/ +#ifdef CONFIG_AB8500_9100_LI_ION_BATTERY +#define BATRES 180 +#else +#define BATRES 300 +#endif I think I mentioned before that you need to get rid of the
CONFIG_AB8500_9100_LI_ION_BATTERYsymbol. If you have exclusive compile-time options, it is impossible to build a kernel that runs on all system, so this has to be a run-time option.Arnd