From: Andrew Morton <akpm@linux-foundation.org>
To: Rodolfo Giometti <giometti@enneenne.com>
Cc: linux-kernel@vger.kernel.org, cbouatmailru@gmail.com,
dwmw2@infradead.org, giometti@linux.it
Subject: Re: [PATCH 1/1] power: support for Texas Instruments BQ27x00 battery managers.
Date: Wed, 13 Aug 2008 16:45:30 -0700 [thread overview]
Message-ID: <20080813164530.82528a4d.akpm@linux-foundation.org> (raw)
In-Reply-To: <1217584435-7337-1-git-send-email-giometti@enneenne.com>
On Fri, 1 Aug 2008 11:53:55 +0200
Rodolfo Giometti <giometti@enneenne.com> wrote:
> From: Rodolfo Giometti <giometti@linux.it>
>
> These battery managers came in two different packages: one for I2C
> busses (BQ27200) and one for HDQ busses (BQ27000).
>
> This driver currently supports only the I2C chip version but the code
> is designed in order to easily allow the HDQ chip version integration.
>
>
> ...
>
> +/* If the system has several batteries we need a different name for each
> + * of them...
> + */
> +DEFINE_IDR(battery_id);
> +DEFINE_MUTEX(battery_mutex);
These should be static.
>
> ...
>
> +static int bq27200_battery_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + char *name;
> + struct bq27x00_device_info *di;
> + struct bq27x00_access_methods *bus;
> + int num, retval = 0;
> +
> + /* Get new ID for the new battery device */
> + retval = idr_pre_get(&battery_id, GFP_KERNEL);
> + if (retval == 0)
> + return -ENOMEM;
> + mutex_lock(&battery_mutex);
> + retval = idr_get_new(&battery_id, client, &num);
> + mutex_unlock(&battery_mutex);
> + if (retval < 0)
> + return retval;
> +
> + name = kmalloc(16, GFP_KERNEL);
> + if (!name) {
> + dev_err(&client->dev, "failed to allocate device name\n");
> + retval = -ENOMEM;
> + goto batt_failed_1;
> + }
> + sprintf(name, "bq27200-%d", num);
Use kasprintf()
> + di = kzalloc(sizeof(*di), GFP_KERNEL);
> + if (!di) {
> + dev_err(&client->dev, "failed to allocate device info data\n");
> + retval = -ENOMEM;
> + goto batt_failed_2;
> + }
> + di->id = num;
> +
Please review:
From: Andrew Morton <akpm@linux-foundation.org>
ERROR: space prohibited before that ':' (ctx:WxE)
#227: FILE: drivers/power/bq27x00_battery.c:175:
+ case POWER_SUPPLY_PROP_VOLTAGE_NOW :
^
ERROR: space prohibited before that ':' (ctx:WxE)
#228: FILE: drivers/power/bq27x00_battery.c:176:
+ case POWER_SUPPLY_PROP_PRESENT :
^
ERROR: space prohibited before that ':' (ctx:WxE)
#235: FILE: drivers/power/bq27x00_battery.c:183:
+ case POWER_SUPPLY_PROP_CURRENT_NOW :
^
ERROR: space prohibited before that ':' (ctx:WxE)
#240: FILE: drivers/power/bq27x00_battery.c:188:
+ case POWER_SUPPLY_PROP_CAPACITY :
^
ERROR: space prohibited before that ':' (ctx:WxE)
#245: FILE: drivers/power/bq27x00_battery.c:193:
+ case POWER_SUPPLY_PROP_TEMP :
^
total: 5 errors, 0 warnings, 412 lines checked
./patches/power-support-for-texas-instruments-bq27x00-battery-managers.patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Please run checkpatch prior to sending patches
Cc: Anton Vorontsov <cbouatmailru@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Rodolfo Giometti <giometti@linux.it>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
drivers/power/bq27x00_battery.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff -puN drivers/power/Kconfig~power-support-for-texas-instruments-bq27x00-battery-managers-checkpatch-fixes drivers/power/Kconfig
diff -puN drivers/power/Makefile~power-support-for-texas-instruments-bq27x00-battery-managers-checkpatch-fixes drivers/power/Makefile
diff -puN drivers/power/bq27x00_battery.c~power-support-for-texas-instruments-bq27x00-battery-managers-checkpatch-fixes drivers/power/bq27x00_battery.c
--- a/drivers/power/bq27x00_battery.c~power-support-for-texas-instruments-bq27x00-battery-managers-checkpatch-fixes
+++ a/drivers/power/bq27x00_battery.c
@@ -172,25 +172,25 @@ static int bq27x00_battery_get_property(
struct bq27x00_device_info *di = to_bq27x00_device_info(psy);
switch (psp) {
- case POWER_SUPPLY_PROP_VOLTAGE_NOW :
- case POWER_SUPPLY_PROP_PRESENT :
+ case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+ case POWER_SUPPLY_PROP_PRESENT:
val->intval = bq27x00_battery_voltage(di);
if (psp == POWER_SUPPLY_PROP_PRESENT)
val->intval = val->intval <= 0 ? 0 : 1;
break;
- case POWER_SUPPLY_PROP_CURRENT_NOW :
+ case POWER_SUPPLY_PROP_CURRENT_NOW:
val->intval = bq27x00_battery_current(di);
break;
- case POWER_SUPPLY_PROP_CAPACITY :
+ case POWER_SUPPLY_PROP_CAPACITY:
val->intval = bq27x00_battery_rsoc(di);
break;
- case POWER_SUPPLY_PROP_TEMP :
+ case POWER_SUPPLY_PROP_TEMP:
val->intval = bq27x00_battery_temperature(di);
break;
_
From: Andrew Morton <akpm@linux-foundation.org>
- make things static
- use kasprintf()
Cc: Anton Vorontsov <cbouatmailru@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Rodolfo Giometti <giometti@linux.it>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
drivers/power/bq27x00_battery.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff -puN drivers/power/Kconfig~power-support-for-texas-instruments-bq27x00-battery-managers-tweaks drivers/power/Kconfig
diff -puN drivers/power/Makefile~power-support-for-texas-instruments-bq27x00-battery-managers-tweaks drivers/power/Makefile
diff -puN drivers/power/bq27x00_battery.c~power-support-for-texas-instruments-bq27x00-battery-managers-tweaks drivers/power/bq27x00_battery.c
--- a/drivers/power/bq27x00_battery.c~power-support-for-texas-instruments-bq27x00-battery-managers-tweaks
+++ a/drivers/power/bq27x00_battery.c
@@ -40,8 +40,8 @@
/* If the system has several batteries we need a different name for each
* of them...
*/
-DEFINE_IDR(battery_id);
-DEFINE_MUTEX(battery_mutex);
+static DEFINE_IDR(battery_id);
+static DEFINE_MUTEX(battery_mutex);
struct bq27x00_device_info;
struct bq27x00_access_methods {
@@ -275,13 +275,12 @@ static int bq27200_battery_probe(struct
if (retval < 0)
return retval;
- name = kmalloc(16, GFP_KERNEL);
+ name = kasprintf(GFP_KERNEL, "bq27200-%d", num);
if (!name) {
dev_err(&client->dev, "failed to allocate device name\n");
retval = -ENOMEM;
goto batt_failed_1;
}
- sprintf(name, "bq27200-%d", num);
di = kzalloc(sizeof(*di), GFP_KERNEL);
if (!di) {
_
prev parent reply other threads:[~2008-08-13 23:46 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-08-01 9:53 [PATCH 1/1] power: support for Texas Instruments BQ27x00 battery managers Rodolfo Giometti
2008-08-13 23:45 ` Andrew Morton [this message]
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=20080813164530.82528a4d.akpm@linux-foundation.org \
--to=akpm@linux-foundation.org \
--cc=cbouatmailru@gmail.com \
--cc=dwmw2@infradead.org \
--cc=giometti@enneenne.com \
--cc=giometti@linux.it \
--cc=linux-kernel@vger.kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox