All of lore.kernel.org
 help / color / mirror / Atom feed
From: lee.jones@linaro.org (Lee Jones)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2] ab8500: remove explicit handling of battery type
Date: Tue, 4 Dec 2012 08:56:47 +0000	[thread overview]
Message-ID: <20121204085647.GA2718@gmail.com> (raw)
In-Reply-To: <1354558375-15110-1-git-send-email-rajanikanth.hv@linaro.org>

> From: "Rajanikanth H.V" <rajanikanth.hv@stericsson.com>
> 
> dt property, 'battery-type' shall be one of supported technology type
> instead blank.
> refer:Documentation/devicetree/bindings/power_supply/ab8500/fg.txt
>       for the list of supported types
> 
> thanks to Francesco Lavra for highlighting missing of_node_put(...)
> 
> for '3.7-rc5': of git://git.infradead.org/battery-2.6.git

Uppercase 'DT'. 
No need for the comma after 'property'.
No real need to reference to the documentation.
No need to add personal thanks to a commit message.
No need to mention the patch's intended route in the commit message.

How about something more like:

Subject: ab8500: Remove initial "UNKNOWN" battery-type string assignment

DT property 'stericsson,battery-type' should always be present in the
Device Tree when describing battery hardware. Thus, instead of
pointlessly assigning "UNKNOWN" as a valid battery-type, we should
actually fail initialisation entirely if the property is missing.

> Acked-by: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Rajanikanth H.V <rajanikanth.hv@stericsson.com>
> ---
>  drivers/power/ab8500_bmdata.c |   11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/power/ab8500_bmdata.c b/drivers/power/ab8500_bmdata.c
> index 03cc528..2230b2c 100644
> --- a/drivers/power/ab8500_bmdata.c
> +++ b/drivers/power/ab8500_bmdata.c
> @@ -461,7 +461,6 @@ bmdevs_of_probe(struct device *dev,
>  	struct  device_node *np_bat_supply;
>  	struct	abx500_bm_data *bat;
>  	const char *btech;
> -	char bat_tech[8];
>  	int i, thermistor;
>  
>  	*battery = &ab8500_bm_data;
> @@ -488,12 +487,10 @@ bmdevs_of_probe(struct device *dev,
>  		"stericsson,battery-type", NULL);
>  	if (!btech) {
>  		dev_warn(dev, "missing property battery-name/type\n");
> -		strcpy(bat_tech, "UNKNOWN");
> -	} else {
> -		strcpy(bat_tech, btech);
> +		of_node_put(np_bat_supply);
> +		return -EINVAL;
>  	}
> -
> -	if (strncmp(bat_tech, "LION", 4) == 0) {
> +	if (strncmp(btech, "LION", 4) == 0) {
>  		bat->no_maintenance  = true;
>  		bat->chg_unknown_bat = true;
>  		bat->bat_type[BATTERY_UNKNOWN].charge_full_design = 2600;
> @@ -508,7 +505,7 @@ bmdevs_of_probe(struct device *dev,
>  		if (thermistor == NTC_EXTERNAL) {
>  			btype->batres_tbl =
>  				temp_to_batres_tbl_ext_thermistor;
> -		} else if (strncmp(bat_tech, "LION", 4) == 0) {
> +		} else if (strncmp(btech, "LION", 4) == 0) {
>  			btype->batres_tbl =
>  				temp_to_batres_tbl_9100;
>  		} else {
> -- 
> 1.7.10.4
> 

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

WARNING: multiple messages have this Message-ID (diff)
From: Lee Jones <lee.jones@linaro.org>
To: "Rajanikanth H.V" <rajanikanth.hv@linaro.org>
Cc: anton.vorontsov@linaro.org, francescolavra.fl@gmail.com,
	rob.herring@calxeda.com, arnd@arndb.de,
	linus.walleij@stericsson.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linaro-dev@lists.linaro.org,
	patches@linaro.org, STEricsson_nomadik_linux@list.st.com,
	rajanikanth.hv@stericsson.com
Subject: Re: [PATCH 1/2] ab8500: remove explicit handling of battery type
Date: Tue, 4 Dec 2012 08:56:47 +0000	[thread overview]
Message-ID: <20121204085647.GA2718@gmail.com> (raw)
In-Reply-To: <1354558375-15110-1-git-send-email-rajanikanth.hv@linaro.org>

> From: "Rajanikanth H.V" <rajanikanth.hv@stericsson.com>
> 
> dt property, 'battery-type' shall be one of supported technology type
> instead blank.
> refer:Documentation/devicetree/bindings/power_supply/ab8500/fg.txt
>       for the list of supported types
> 
> thanks to Francesco Lavra for highlighting missing of_node_put(...)
> 
> for '3.7-rc5': of git://git.infradead.org/battery-2.6.git

Uppercase 'DT'. 
No need for the comma after 'property'.
No real need to reference to the documentation.
No need to add personal thanks to a commit message.
No need to mention the patch's intended route in the commit message.

How about something more like:

Subject: ab8500: Remove initial "UNKNOWN" battery-type string assignment

DT property 'stericsson,battery-type' should always be present in the
Device Tree when describing battery hardware. Thus, instead of
pointlessly assigning "UNKNOWN" as a valid battery-type, we should
actually fail initialisation entirely if the property is missing.

> Acked-by: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Rajanikanth H.V <rajanikanth.hv@stericsson.com>
> ---
>  drivers/power/ab8500_bmdata.c |   11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/power/ab8500_bmdata.c b/drivers/power/ab8500_bmdata.c
> index 03cc528..2230b2c 100644
> --- a/drivers/power/ab8500_bmdata.c
> +++ b/drivers/power/ab8500_bmdata.c
> @@ -461,7 +461,6 @@ bmdevs_of_probe(struct device *dev,
>  	struct  device_node *np_bat_supply;
>  	struct	abx500_bm_data *bat;
>  	const char *btech;
> -	char bat_tech[8];
>  	int i, thermistor;
>  
>  	*battery = &ab8500_bm_data;
> @@ -488,12 +487,10 @@ bmdevs_of_probe(struct device *dev,
>  		"stericsson,battery-type", NULL);
>  	if (!btech) {
>  		dev_warn(dev, "missing property battery-name/type\n");
> -		strcpy(bat_tech, "UNKNOWN");
> -	} else {
> -		strcpy(bat_tech, btech);
> +		of_node_put(np_bat_supply);
> +		return -EINVAL;
>  	}
> -
> -	if (strncmp(bat_tech, "LION", 4) == 0) {
> +	if (strncmp(btech, "LION", 4) == 0) {
>  		bat->no_maintenance  = true;
>  		bat->chg_unknown_bat = true;
>  		bat->bat_type[BATTERY_UNKNOWN].charge_full_design = 2600;
> @@ -508,7 +505,7 @@ bmdevs_of_probe(struct device *dev,
>  		if (thermistor == NTC_EXTERNAL) {
>  			btype->batres_tbl =
>  				temp_to_batres_tbl_ext_thermistor;
> -		} else if (strncmp(bat_tech, "LION", 4) == 0) {
> +		} else if (strncmp(btech, "LION", 4) == 0) {
>  			btype->batres_tbl =
>  				temp_to_batres_tbl_9100;
>  		} else {
> -- 
> 1.7.10.4
> 

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

  parent reply	other threads:[~2012-12-04  8:56 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-03 18:12 [PATCH 1/2] ab8500: remove explicit handling of battery type Rajanikanth H.V
2012-12-03 18:12 ` Rajanikanth H.V
2012-12-03 18:12 ` [PATCH 2/2] ab8500: promote ab8500_fg probe before ab8500_btemp probe Rajanikanth H.V
2012-12-03 18:12   ` Rajanikanth H.V
2013-01-06  1:40   ` Anton Vorontsov
2013-01-06  1:40     ` Anton Vorontsov
2012-12-04  8:56 ` Lee Jones [this message]
2012-12-04  8:56   ` [PATCH 1/2] ab8500: remove explicit handling of battery type Lee Jones

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121204085647.GA2718@gmail.com \
    --to=lee.jones@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.