Re: [PATCH] mfd: Implement devicetree support for AB8500 Btemp
@@ -964,11 +966,13 @@ static int __devinit ab8500_btemp_probe(struct
platform_device *pdev)
{
int irq, i, ret = 0;
u8 val;
- struct abx500_bm_plat_data *plat_data = pdev->dev.platform_data;
I already told you about this.
your previous comment was: "No, it's meant to work with _both_ platform
and Device Tree registation."
Right.
Why is this required when platform specific information/file is removed?
It's not being removed, it's just not allowed to be upstreamed. Once we
enforce Device Tree only booting, we'll go round and remove that
capability from the drivers. Until then, you must ensure all Device Tree
enablement works in parallel with platform registration.
Concentrate on enabling the drivers for DT for now and we'll extract
platform data support later.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
M: +44 77 88 633 515
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mfd: Implement devicetree support for AB8500 Btemp
> Date: Tue, 10 Jul 2012 14:12:01 + > From: Arnd Bergmann > To: linux-arm-ker...@lists.infradead.org > Cc: linaro-...@lists.linaro.org, linux-kernel@vger.kernel.org, > "Rajanikanth H.V" , patc...@linaro.org > Subject: Re: [PATCH] mfd: Implement devicetree support for AB8500 > Btemp > Message-ID: <201207101412.01561.a...@arndb.de> > Content-Type: Text/Plain; charset="utf-8" > > On Tuesday 10 July 2012, Rajanikanth H.V wrote: > >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt >> @@ -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 >> + - 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. > >> +Supplied-to: >> + This shall be power supply class dependency where in the runtime battery >> + properties will be shared across fuel guage and charging algorithm driver. > > I probably don't understand enough of this, but shouldn't the other devices > 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.c Need 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? > > You are also not listing some of the properties that are in the device > tree here, like the "interrupts" property itself. > >> diff --git a/arch/arm/mach-ux500/board-mop500-bm.c b/arch/arm/mach-ux500/board-mop500-bm.c >> new file mode 100644 >> index 000..3349ceb >> --- /dev/null >> +++ b/arch/arm/mach-ux500/board-mop500-bm.c > > Didn'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. > >> +/* >> + * 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_BATTERY symbol. 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 > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mfd: Implement devicetree support for AB8500 Btemp
> Date: Tue, 10 Jul 2012 18:20:13 +0200
> From: Lee Jones
> To: "Rajanikanth H.V"
> Cc: STEricsson_nomadik_linux ,
> linaro-...@lists.linaro.org, linux-kernel@vger.kernel.org,
> linux-arm-ker...@lists.infradead.org, patc...@linaro.org
> Subject: Re: [PATCH] mfd: Implement devicetree support for AB8500
> Btemp
> Message-ID: <4ffc563d.2080...@linaro.org>
> Content-Type: text/plain; charset=UTF-8; format=flowed
>
> Some of my comments are picky, but if it's going into Mainline, it
> should at least look professional.
>
> You didn't CC the ST-Ericsson internal mailing list.
>
> Address is stericsson_nomadik_li...@list.st.com
>
> I'll do it for now, but please do so on your next submission.
>
> On 10/07/12 15:23, Rajanikanth H.V wrote:
>> From: "Rajanikanth H.V"
>>
>> This patch adds device tree support for
>> battery temperature monitor driver
>>
>> Signed-off-by: Rajanikanth H.V
>> ---
>> .../bindings/power_supply/ab8500/btemp.txt | 54 ++
>> arch/arm/boot/dts/db8500.dtsi | 17 +
>> arch/arm/mach-ux500/Makefile |4 +-
>> arch/arm/mach-ux500/board-mop500-bm.c | 532
>>
>> arch/arm/mach-ux500/include/mach/board-mop500-bm.h | 24 +
>> drivers/mfd/ab8500-core.c |1 +
>> drivers/power/Kconfig |8 +-
>> drivers/power/ab8500_btemp.c | 79 ++-
>> include/linux/mfd/ab8500/pwmleds.h | 20 +
>> 9 files changed, 722 insertions(+), 17 deletions(-)
>> create mode 100644
>> Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt
>> create mode 100644 arch/arm/mach-ux500/board-mop500-bm.c
>> create mode 100644 arch/arm/mach-ux500/include/mach/board-mop500-bm.h
>> create mode 100644 include/linux/mfd/ab8500/pwmleds.h
>>
>> diff --git a/Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt
>> b/Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt
>> new file mode 100644
>> index 000..8543ed1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt
>> @@ -0,0 +1,54 @@
>> +AB8500 Battery Termperature Monitor Driver
>
> Spelling.
>
>> +
>> +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.
>
> Mixture of capitalised and otherwise device names.
>
>> +
>> +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.
>
> Spelling. Mixed use of ''.
>
>> +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
>> + - BtempLow
>> + - BtempMedium
>> + - BtempHigh
>> +
>> +
>> +Supplied-to:
>> + This shall be power supply class dependency where in the runtime
>> battery
>> + properties will be shared across fuel guage and charging algorithm
>> driver.
>
> Spelling.
>
>> +
>> +Thermister-interface:
>> + 'btemp' and 'batctrl' are the pins interfaced for battery temperature
>> + measurement, btemp is used when NTC(Negative Termperature coefficient)
>
> Spelling.
>
>> + resister is interfaced external to battery and batctrl is used when
>> + NTC resister is internal to battery.
>> +
>> +example:
>> +ab8500-btemp {
>> + compatible = "stericsson,ab8500-btemp";
>> +
>> + interrupt-names =
>> + "BAT_CTRL_INDB",/* battery removal indicator */
>> + "BTEMP_LOW",/* Btemp < BtempLow, if battery
>> temperature is lower than -10?C */
>> + "BTEMP_HIGH", /* BtempLow < Btemp < BtempMedium,
>> + if battery
>> temperature is between -10 and 0?C */
>> + "BTEMP_LOW_MEDIUM", /* BtempMedium < Btemp < BtempHigh,
>> +
Re: [PATCH] mfd: Implement devicetree support for AB8500 Btemp
Date: Tue, 10 Jul 2012 18:20:13 +0200
From: Lee Jones lee.jo...@linaro.org
To: Rajanikanth H.V rajanikanth...@stericsson.com
Cc: STEricsson_nomadik_linux stericsson_nomadik_li...@list.st.com,
linaro-...@lists.linaro.org, linux-kernel@vger.kernel.org,
linux-arm-ker...@lists.infradead.org, patc...@linaro.org
Subject: Re: [PATCH] mfd: Implement devicetree support for AB8500
Btemp
Message-ID: 4ffc563d.2080...@linaro.org
Content-Type: text/plain; charset=UTF-8; format=flowed
Some of my comments are picky, but if it's going into Mainline, it
should at least look professional.
You didn't CC the ST-Ericsson internal mailing list.
Address is stericsson_nomadik_li...@list.st.com
I'll do it for now, but please do so on your next submission.
On 10/07/12 15:23, Rajanikanth H.V wrote:
From: Rajanikanth H.V rajanikanth...@stericsson.com
This patch adds device tree support for
battery temperature monitor driver
Signed-off-by: Rajanikanth H.V rajanikanth...@stericsson.com
---
.../bindings/power_supply/ab8500/btemp.txt | 54 ++
arch/arm/boot/dts/db8500.dtsi | 17 +
arch/arm/mach-ux500/Makefile |4 +-
arch/arm/mach-ux500/board-mop500-bm.c | 532
arch/arm/mach-ux500/include/mach/board-mop500-bm.h | 24 +
drivers/mfd/ab8500-core.c |1 +
drivers/power/Kconfig |8 +-
drivers/power/ab8500_btemp.c | 79 ++-
include/linux/mfd/ab8500/pwmleds.h | 20 +
9 files changed, 722 insertions(+), 17 deletions(-)
create mode 100644
Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt
create mode 100644 arch/arm/mach-ux500/board-mop500-bm.c
create mode 100644 arch/arm/mach-ux500/include/mach/board-mop500-bm.h
create mode 100644 include/linux/mfd/ab8500/pwmleds.h
diff --git a/Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt
b/Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt
new file mode 100644
index 000..8543ed1
--- /dev/null
+++ b/Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt
@@ -0,0 +1,54 @@
+AB8500 Battery Termperature Monitor Driver
Spelling.
+
+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.
Mixture of capitalised and otherwise device names.
+
+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.
Spelling. Mixed use of ''.
+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
+ - BtempLow
+ - BtempMedium
+ - BtempHigh
+
+
+Supplied-to:
+ This shall be power supply class dependency where in the runtime
battery
+ properties will be shared across fuel guage and charging algorithm
driver.
Spelling.
+
+Thermister-interface:
+ 'btemp' and 'batctrl' are the pins interfaced for battery temperature
+ measurement, btemp is used when NTC(Negative Termperature coefficient)
Spelling.
+ resister is interfaced external to battery and batctrl is used when
+ NTC resister is internal to battery.
+
+example:
+ab8500-btemp {
+ compatible = stericsson,ab8500-btemp;
+
+ interrupt-names =
+ BAT_CTRL_INDB,/* battery removal indicator */
+ BTEMP_LOW,/* Btemp BtempLow, if battery
temperature is lower than -10?C */
+ BTEMP_HIGH, /* BtempLow Btemp BtempMedium,
+ if battery
temperature is between -10 and 0?C */
+ BTEMP_LOW_MEDIUM, /* BtempMedium Btemp BtempHigh,
+ if battery
temperature is between 0?C and ?MaxTemp?.*/
+ BTEMP_MEDIUM_HIGH;/* Btemp BtempHigh,
+ if battery
temperature is higher than ?MaxTemp?. */
+
+ supplied-to = ab8500_chargalg, ab8500_fg;
+
+ thermister-internal-to-battery = 1;
+};
diff --git a/arch/arm/boot/dts/db8500.dtsi b/arch/arm/boot/dts/db8500.dtsi
index 7279165..527fdc3 100644
--- a/arch/arm/boot/dts/db8500.dtsi
+++ b/arch/arm/boot/dts/db8500.dtsi
@@ -330,6 +330,23 @@
vddadc-supply =
ab8500_ldo_tvout_reg;
};
+ ab8500-btemp
Re: [PATCH] mfd: Implement devicetree support for AB8500 Btemp
Date: Tue, 10 Jul 2012 14:12:01 + From: Arnd Bergmann a...@arndb.de To: linux-arm-ker...@lists.infradead.org Cc: linaro-...@lists.linaro.org, linux-kernel@vger.kernel.org, Rajanikanth H.V rajanikanth...@stericsson.com, patc...@linaro.org Subject: Re: [PATCH] mfd: Implement devicetree support for AB8500 Btemp Message-ID: 201207101412.01561.a...@arndb.de Content-Type: Text/Plain; charset=utf-8 On Tuesday 10 July 2012, Rajanikanth H.V wrote: --- /dev/null +++ b/Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt @@ -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 + - 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. +Supplied-to: + This shall be power supply class dependency where in the runtime battery + properties will be shared across fuel guage and charging algorithm driver. I probably don't understand enough of this, but shouldn't the other devices 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.c Need 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? You are also not listing some of the properties that are in the device tree here, like the interrupts property itself. diff --git a/arch/arm/mach-ux500/board-mop500-bm.c b/arch/arm/mach-ux500/board-mop500-bm.c new file mode 100644 index 000..3349ceb --- /dev/null +++ b/arch/arm/mach-ux500/board-mop500-bm.c Didn'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. +/* + * 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_BATTERY symbol. 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mfd: Implement devicetree support for AB8500 Btemp
@@ -964,11 +966,13 @@ static int __devinit ab8500_btemp_probe(struct
platform_device *pdev)
{
int irq, i, ret = 0;
u8 val;
- struct abx500_bm_plat_data *plat_data = pdev-dev.platform_data;
I already told you about this.
your previous comment was: No, it's meant to work with _both_ platform
and Device Tree registation.
Right.
Why is this required when platform specific information/file is removed?
It's not being removed, it's just not allowed to be upstreamed. Once we
enforce Device Tree only booting, we'll go round and remove that
capability from the drivers. Until then, you must ensure all Device Tree
enablement works in parallel with platform registration.
Concentrate on enabling the drivers for DT for now and we'll extract
platform data support later.
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
M: +44 77 88 633 515
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mfd: Implement devicetree support for AB8500 Btemp
Some of my comments are picky, but if it's going into Mainline, it
should at least look professional.
You didn't CC the ST-Ericsson internal mailing list.
Address is stericsson_nomadik_li...@list.st.com
I'll do it for now, but please do so on your next submission.
On 10/07/12 15:23, Rajanikanth H.V wrote:
From: "Rajanikanth H.V"
This patch adds device tree support for
battery temperature monitor driver
Signed-off-by: Rajanikanth H.V
---
.../bindings/power_supply/ab8500/btemp.txt | 54 ++
arch/arm/boot/dts/db8500.dtsi | 17 +
arch/arm/mach-ux500/Makefile |4 +-
arch/arm/mach-ux500/board-mop500-bm.c | 532
arch/arm/mach-ux500/include/mach/board-mop500-bm.h | 24 +
drivers/mfd/ab8500-core.c |1 +
drivers/power/Kconfig |8 +-
drivers/power/ab8500_btemp.c | 79 ++-
include/linux/mfd/ab8500/pwmleds.h | 20 +
9 files changed, 722 insertions(+), 17 deletions(-)
create mode 100644
Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt
create mode 100644 arch/arm/mach-ux500/board-mop500-bm.c
create mode 100644 arch/arm/mach-ux500/include/mach/board-mop500-bm.h
create mode 100644 include/linux/mfd/ab8500/pwmleds.h
diff --git a/Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt
b/Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt
new file mode 100644
index 000..8543ed1
--- /dev/null
+++ b/Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt
@@ -0,0 +1,54 @@
+AB8500 Battery Termperature Monitor Driver
Spelling.
+
+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.
Mixture of capitalised and otherwise device names.
+
+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.
Spelling. Mixed use of ''.
+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
+ - BtempLow
+ - BtempMedium
+ - BtempHigh
+
+
+Supplied-to:
+ This shall be power supply class dependency where in the runtime battery
+ properties will be shared across fuel guage and charging algorithm
driver.
Spelling.
+
+Thermister-interface:
+ 'btemp' and 'batctrl' are the pins interfaced for battery temperature
+ measurement, btemp is used when NTC(Negative Termperature coefficient)
Spelling.
+ resister is interfaced external to battery and batctrl is used when
+ NTC resister is internal to battery.
+
+example:
+ab8500-btemp {
+ compatible = "stericsson,ab8500-btemp";
+
+ interrupt-names =
+ "BAT_CTRL_INDB", /* battery removal indicator */
+ "BTEMP_LOW", /* Btemp < BtempLow, if battery
temperature is lower than -10°C */
+ "BTEMP_HIGH", /* BtempLow < Btemp < BtempMedium,
+ if battery
temperature is between -10 and 0°C */
+ "BTEMP_LOW_MEDIUM", /* BtempMedium < Btemp < BtempHigh,
+ if battery
temperature is between 0°C and “MaxTemp”.*/
+ "BTEMP_MEDIUM_HIGH";/* Btemp > BtempHigh,
+ if battery
temperature is higher than “MaxTemp”. */
+
+ supplied-to = "ab8500_chargalg", "ab8500_fg";
+
+ thermister-internal-to-battery = <1>;
+};
diff --git a/arch/arm/boot/dts/db8500.dtsi b/arch/arm/boot/dts/db8500.dtsi
index 7279165..527fdc3 100644
--- a/arch/arm/boot/dts/db8500.dtsi
+++ b/arch/arm/boot/dts/db8500.dtsi
@@ -330,6 +330,23 @@
vddadc-supply = <_ldo_tvout_reg>;
};
+ ab8500-btemp {
+ compatible = "stericsson,ab8500-btemp";
+ interrupts = <20 0x04
+ 80 0x04
+ 81 0x04
+ 82 0x04
+ 83 0x04>;
Odd tabbing.
+ interrupt-names = "BAT_CTRL_INDB",
+
"BTEMP_LOW",
+
Re: [PATCH] mfd: Implement devicetree support for AB8500 Btemp
On Tuesday 10 July 2012, Rajanikanth H.V wrote: > --- /dev/null > +++ b/Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt > @@ -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 > + - 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. > +Supplied-to: > + This shall be power supply class dependency where in the runtime battery > + properties will be shared across fuel guage and charging algorithm > driver. I probably don't understand enough of this, but shouldn't the other devices 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? You are also not listing some of the properties that are in the device tree here, like the "interrupts" property itself. > diff --git a/arch/arm/mach-ux500/board-mop500-bm.c > b/arch/arm/mach-ux500/board-mop500-bm.c > new file mode 100644 > index 000..3349ceb > --- /dev/null > +++ b/arch/arm/mach-ux500/board-mop500-bm.c Didn'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. > +/* > + * 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_BATTERY symbol. 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mfd: Implement devicetree support for AB8500 Btemp
On Tuesday 10 July 2012, Rajanikanth H.V wrote: --- /dev/null +++ b/Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt @@ -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 + - 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. +Supplied-to: + This shall be power supply class dependency where in the runtime battery + properties will be shared across fuel guage and charging algorithm driver. I probably don't understand enough of this, but shouldn't the other devices 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? You are also not listing some of the properties that are in the device tree here, like the interrupts property itself. diff --git a/arch/arm/mach-ux500/board-mop500-bm.c b/arch/arm/mach-ux500/board-mop500-bm.c new file mode 100644 index 000..3349ceb --- /dev/null +++ b/arch/arm/mach-ux500/board-mop500-bm.c Didn'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. +/* + * 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_BATTERY symbol. 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mfd: Implement devicetree support for AB8500 Btemp
Some of my comments are picky, but if it's going into Mainline, it
should at least look professional.
You didn't CC the ST-Ericsson internal mailing list.
Address is stericsson_nomadik_li...@list.st.com
I'll do it for now, but please do so on your next submission.
On 10/07/12 15:23, Rajanikanth H.V wrote:
From: Rajanikanth H.V rajanikanth...@stericsson.com
This patch adds device tree support for
battery temperature monitor driver
Signed-off-by: Rajanikanth H.V rajanikanth...@stericsson.com
---
.../bindings/power_supply/ab8500/btemp.txt | 54 ++
arch/arm/boot/dts/db8500.dtsi | 17 +
arch/arm/mach-ux500/Makefile |4 +-
arch/arm/mach-ux500/board-mop500-bm.c | 532
arch/arm/mach-ux500/include/mach/board-mop500-bm.h | 24 +
drivers/mfd/ab8500-core.c |1 +
drivers/power/Kconfig |8 +-
drivers/power/ab8500_btemp.c | 79 ++-
include/linux/mfd/ab8500/pwmleds.h | 20 +
9 files changed, 722 insertions(+), 17 deletions(-)
create mode 100644
Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt
create mode 100644 arch/arm/mach-ux500/board-mop500-bm.c
create mode 100644 arch/arm/mach-ux500/include/mach/board-mop500-bm.h
create mode 100644 include/linux/mfd/ab8500/pwmleds.h
diff --git a/Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt
b/Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt
new file mode 100644
index 000..8543ed1
--- /dev/null
+++ b/Documentation/devicetree/bindings/power_supply/ab8500/btemp.txt
@@ -0,0 +1,54 @@
+AB8500 Battery Termperature Monitor Driver
Spelling.
+
+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.
Mixture of capitalised and otherwise device names.
+
+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.
Spelling. Mixed use of ''.
+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
+ - BtempLow
+ - BtempMedium
+ - BtempHigh
+
+
+Supplied-to:
+ This shall be power supply class dependency where in the runtime battery
+ properties will be shared across fuel guage and charging algorithm
driver.
Spelling.
+
+Thermister-interface:
+ 'btemp' and 'batctrl' are the pins interfaced for battery temperature
+ measurement, btemp is used when NTC(Negative Termperature coefficient)
Spelling.
+ resister is interfaced external to battery and batctrl is used when
+ NTC resister is internal to battery.
+
+example:
+ab8500-btemp {
+ compatible = stericsson,ab8500-btemp;
+
+ interrupt-names =
+ BAT_CTRL_INDB, /* battery removal indicator */
+ BTEMP_LOW, /* Btemp BtempLow, if battery
temperature is lower than -10°C */
+ BTEMP_HIGH, /* BtempLow Btemp BtempMedium,
+ if battery
temperature is between -10 and 0°C */
+ BTEMP_LOW_MEDIUM, /* BtempMedium Btemp BtempHigh,
+ if battery
temperature is between 0°C and “MaxTemp”.*/
+ BTEMP_MEDIUM_HIGH;/* Btemp BtempHigh,
+ if battery
temperature is higher than “MaxTemp”. */
+
+ supplied-to = ab8500_chargalg, ab8500_fg;
+
+ thermister-internal-to-battery = 1;
+};
diff --git a/arch/arm/boot/dts/db8500.dtsi b/arch/arm/boot/dts/db8500.dtsi
index 7279165..527fdc3 100644
--- a/arch/arm/boot/dts/db8500.dtsi
+++ b/arch/arm/boot/dts/db8500.dtsi
@@ -330,6 +330,23 @@
vddadc-supply = ab8500_ldo_tvout_reg;
};
+ ab8500-btemp {
+ compatible = stericsson,ab8500-btemp;
+ interrupts = 20 0x04
+ 80 0x04
+ 81 0x04
+ 82 0x04
+ 83 0x04;
Odd tabbing.
+ interrupt-names = BAT_CTRL_INDB,
+
BTEMP_LOW,
+
