* [PATCH] pda_power: clean up irq, timer, return usage
@ 2007-07-14 23:12 Jeff Garzik
2007-07-15 0:15 ` Anton Vorontsov
2007-07-15 1:02 ` [PATCH] MAINTAINERS: Add maintainers for power supply subsystem and drivers (was [PATCH] pda_power: clean up irq, timer, return usage) Anton Vorontsov
0 siblings, 2 replies; 9+ messages in thread
From: Jeff Garzik @ 2007-07-14 23:12 UTC (permalink / raw)
To: Andrew Morton, Anton Vorontsov; +Cc: LKML
Clean up pda_power interrupt handling:
Prior to this patch, the driver would pass information it needed
to the interrupt handler dev_id pointer, and then prompt forget it
ever did so, recreating that same information after a couple passes
through the timer-based state machine.
This patch removes the redundant checks by passing the
pda_power_supply[] pointer through the state machine. The current
code passed 'irq' through the state machine, as an index to recreate
the pointer, when we could more simply pass around the pointer itself.
Additionally, bogus "return;" statements were removed.
This patch makes it easier to remove the 'irq' argument in the future,
in addition to cleaning up the driver today.
P.S. Where are the MAINTAINERS entries for this driver, and
drivers/power in general?
Signed-off-by: Jeff Garzik <jeff@garzik.org>
diff --git a/drivers/power/pda_power.c b/drivers/power/pda_power.c
index 4e1eb04..dacbe50 100644
--- a/drivers/power/pda_power.c
+++ b/drivers/power/pda_power.c
@@ -97,36 +97,31 @@ static void update_charger(void)
dev_dbg(dev, "charger off\n");
pdata->set_charge(0);
}
-
- return;
}
-static void supply_timer_func(unsigned long irq)
+static void supply_timer_func(unsigned long power_supply_ptr)
{
- if (ac_irq && irq == ac_irq->start)
- power_supply_changed(&pda_power_supplies[0]);
- else if (usb_irq && irq == usb_irq->start)
- power_supply_changed(&pda_power_supplies[1]);
- return;
+ void *power_supply = (void *) power_supply_ptr;
+
+ power_supply_changed(power_supply);
}
-static void charger_timer_func(unsigned long irq)
+static void charger_timer_func(unsigned long power_supply_ptr)
{
update_charger();
/* Okay, charger set. Now wait a bit before notifying supplicants,
* charge power should stabilize. */
- supply_timer.data = irq;
+ supply_timer.data = power_supply_ptr;
mod_timer(&supply_timer,
jiffies + msecs_to_jiffies(pdata->wait_for_charger));
- return;
}
-static irqreturn_t power_changed_isr(int irq, void *unused)
+static irqreturn_t power_changed_isr(int irq, void *power_supply)
{
/* Wait a bit before reading ac/usb line status and setting charger,
* because ac/usb status readings may lag from irq. */
- charger_timer.data = irq;
+ charger_timer.data = (unsigned long) power_supply;
mod_timer(&charger_timer,
jiffies + msecs_to_jiffies(pdata->wait_for_status));
return IRQ_HANDLED;
@@ -252,7 +247,6 @@ static int __init pda_power_init(void)
static void __exit pda_power_exit(void)
{
platform_driver_unregister(&pda_power_pdrv);
- return;
}
module_init(pda_power_init);
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] pda_power: clean up irq, timer, return usage
2007-07-14 23:12 [PATCH] pda_power: clean up irq, timer, return usage Jeff Garzik
@ 2007-07-15 0:15 ` Anton Vorontsov
2007-07-15 0:29 ` Jeff Garzik
2007-07-15 1:02 ` [PATCH] MAINTAINERS: Add maintainers for power supply subsystem and drivers (was [PATCH] pda_power: clean up irq, timer, return usage) Anton Vorontsov
1 sibling, 1 reply; 9+ messages in thread
From: Anton Vorontsov @ 2007-07-15 0:15 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Andrew Morton, LKML
On Sat, Jul 14, 2007 at 07:12:04PM -0400, Jeff Garzik wrote:
>
> Clean up pda_power interrupt handling:
Nice, thanks! Just few cosmetic comments.
> Prior to this patch, the driver would pass information it needed
> to the interrupt handler dev_id pointer, and then prompt forget it
> ever did so, recreating that same information after a couple passes
> through the timer-based state machine.
>
> This patch removes the redundant checks by passing the
> pda_power_supply[] pointer through the state machine. The current
> code passed 'irq' through the state machine, as an index to recreate
> the pointer, when we could more simply pass around the pointer itself.
>
> Additionally, bogus "return;" statements were removed.
My preference is to use "return;" statements in functions returning
void, even if functions is very small. It's just sugar for my eyes,
to really see exit points. . Without returns I've feeling that
something is missing there. Yes, my oddity.
Plus, so far CodingStyle does not say anything about non obligatory
return statements.
These should be "fixed" too, though:
~/linux-2.6$ grep -h "return;" -A1 -r drivers/ arch/ | grep "^}$" | \
wc -l
1354
Obviously, drivers/ata is almost pure (3). ;-)
Either way, I prefer to leave alone these "return;"s, until CodingStyle
permits them.
> This patch makes it easier to remove the 'irq' argument in the future,
> in addition to cleaning up the driver today.
>
> P.S. Where are the MAINTAINERS entries for this driver, and
> drivers/power in general?
I'll send patch shortly.
> + void *power_supply = (void *) power_supply_ptr;
^
I guess common practice is not to put space here.
> + charger_timer.data = (unsigned long) power_supply;
And here.
I'll correct this, and will apply to battery2-2.6.git.
Thanks!
--
Anton Vorontsov
email: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] pda_power: clean up irq, timer, return usage
2007-07-15 0:15 ` Anton Vorontsov
@ 2007-07-15 0:29 ` Jeff Garzik
2007-07-15 1:43 ` [PATCH] pda_power: clean up irq, timer Anton Vorontsov
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Jeff Garzik @ 2007-07-15 0:29 UTC (permalink / raw)
To: cbou; +Cc: Andrew Morton, LKML
Anton Vorontsov wrote:
> On Sat, Jul 14, 2007 at 07:12:04PM -0400, Jeff Garzik wrote:
>> Clean up pda_power interrupt handling:
>
> Nice, thanks! Just few cosmetic comments.
>
>> Prior to this patch, the driver would pass information it needed
>> to the interrupt handler dev_id pointer, and then prompt forget it
>> ever did so, recreating that same information after a couple passes
>> through the timer-based state machine.
>>
>> This patch removes the redundant checks by passing the
>> pda_power_supply[] pointer through the state machine. The current
>> code passed 'irq' through the state machine, as an index to recreate
>> the pointer, when we could more simply pass around the pointer itself.
>>
>> Additionally, bogus "return;" statements were removed.
>
> My preference is to use "return;" statements in functions returning
> void, even if functions is very small. It's just sugar for my eyes,
> to really see exit points. . Without returns I've feeling that
> something is missing there. Yes, my oddity.
>
> Plus, so far CodingStyle does not say anything about non obligatory
> return statements.
>
> These should be "fixed" too, though:
> ~/linux-2.6$ grep -h "return;" -A1 -r drivers/ arch/ | grep "^}$" | \
> wc -l
> 1354
>
> Obviously, drivers/ata is almost pure (3). ;-)
>
> Either way, I prefer to leave alone these "return;"s, until CodingStyle
> permits them.
CodingStyle is not the end-all of rulebooks. See repeated messages by
Linus, me, and others on the subject. CodingStyle intentionally does
not list a rule for every possible C code incarnation.
It's the general style to avoid these return statements in new code. It
makes the page longer, meaning less code to review. It tends to trip up
people making bombing runs through drivers, and searching for return
statements in their favorite editor.
If you consider how many void functions there are in the kernel, 1354 is
the obvious minority, being far smaller than "millions." Please follow
what -millions- of void functions already do in the kernel.
>> This patch makes it easier to remove the 'irq' argument in the future,
>> in addition to cleaning up the driver today.
>>
>> P.S. Where are the MAINTAINERS entries for this driver, and
>> drivers/power in general?
>
> I'll send patch shortly.
>
>> + void *power_supply = (void *) power_supply_ptr;
> ^
> I guess common practice is not to put space here.
incorrect. a space goes there, as I put it.
>> + charger_timer.data = (unsigned long) power_supply;
>
> And here.
ditto.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] MAINTAINERS: Add maintainers for power supply subsystem and drivers (was [PATCH] pda_power: clean up irq, timer, return usage)
2007-07-14 23:12 [PATCH] pda_power: clean up irq, timer, return usage Jeff Garzik
2007-07-15 0:15 ` Anton Vorontsov
@ 2007-07-15 1:02 ` Anton Vorontsov
2007-07-15 7:54 ` David Woodhouse
1 sibling, 1 reply; 9+ messages in thread
From: Anton Vorontsov @ 2007-07-15 1:02 UTC (permalink / raw)
To: David Woodhouse; +Cc: Andrew Morton, LKML, Jeff Garzik
On Sat, Jul 14, 2007 at 07:12:04PM -0400, Jeff Garzik wrote:
> P.S. Where are the MAINTAINERS entries for this driver, and
> drivers/power in general?
Collecting acks. David?
From: Anton Vorontsov <cbou@mail.ru>
Subject: [PATCH] MAINTAINERS: Add maintainers for power supply subsystem and drivers
Signed-off-by: Anton Vorontsov <cbou@mail.ru>
---
MAINTAINERS | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 845fbf4..3ccefac 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2863,6 +2863,17 @@ M: george@mvista.com
L: linux-kernel@vger.kernel.org
S: Supported
+POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS
+P: Anton Vorontsov
+M: cbou@mail.ru
+P: David Woodhouse
+M: dwmw2@infradead.org
+L: linux-kernel@vger.kernel.org
+L: kernel-discuss@handhelds.org
+T: git git.infradead.org/~cbou/battery2-2.6.git
+T: git git.infradead.org/~dwmw2/battery-2.6.git
+S: Maintained
+
POWERPC 4xx EMAC DRIVER
P: Eugene Surovegin
M: ebs@ebshome.net
--
1.5.1.1-dirty
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH] pda_power: clean up irq, timer
2007-07-15 0:29 ` Jeff Garzik
@ 2007-07-15 1:43 ` Anton Vorontsov
2007-07-15 1:45 ` [PATCH] Power supply class and drivers: remove non obligatory return statements Anton Vorontsov
2007-07-15 4:50 ` [PATCH] pda_power: clean up irq, timer, return usage Andrew Morton
2 siblings, 0 replies; 9+ messages in thread
From: Anton Vorontsov @ 2007-07-15 1:43 UTC (permalink / raw)
To: Jeff Garzik; +Cc: linux-kernel, Andrew Morton, David Woodhouse
Clean up pda_power interrupt handling:
Prior to this patch, the driver would pass information it needed
to the interrupt handler dev_id pointer, and then prompt forget it
ever did so, recreating that same information after a couple passes
through the timer-based state machine.
This patch removes the redundant checks by passing the
pda_power_supply[] pointer through the state machine. The current
code passed 'irq' through the state machine, as an index to recreate
the pointer, when we could more simply pass around the pointer itself.
This patch makes it easier to remove the 'irq' argument in the future,
in addition to cleaning up the driver today.
Signed-off-by: Jeff Garzik <jeff@garzik.org>
---
drivers/power/pda_power.c | 17 ++++++++---------
1 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/power/pda_power.c b/drivers/power/pda_power.c
index 4e1eb04..8304506 100644
--- a/drivers/power/pda_power.c
+++ b/drivers/power/pda_power.c
@@ -101,32 +101,31 @@ static void update_charger(void)
return;
}
-static void supply_timer_func(unsigned long irq)
+static void supply_timer_func(unsigned long power_supply_ptr)
{
- if (ac_irq && irq == ac_irq->start)
- power_supply_changed(&pda_power_supplies[0]);
- else if (usb_irq && irq == usb_irq->start)
- power_supply_changed(&pda_power_supplies[1]);
+ void *power_supply = (void *) power_supply_ptr;
+
+ power_supply_changed(power_supply);
return;
}
-static void charger_timer_func(unsigned long irq)
+static void charger_timer_func(unsigned long power_supply_ptr)
{
update_charger();
/* Okay, charger set. Now wait a bit before notifying supplicants,
* charge power should stabilize. */
- supply_timer.data = irq;
+ supply_timer.data = power_supply_ptr;
mod_timer(&supply_timer,
jiffies + msecs_to_jiffies(pdata->wait_for_charger));
return;
}
-static irqreturn_t power_changed_isr(int irq, void *unused)
+static irqreturn_t power_changed_isr(int irq, void *power_supply)
{
/* Wait a bit before reading ac/usb line status and setting charger,
* because ac/usb status readings may lag from irq. */
- charger_timer.data = irq;
+ charger_timer.data = (unsigned long) power_supply;
mod_timer(&charger_timer,
jiffies + msecs_to_jiffies(pdata->wait_for_status));
return IRQ_HANDLED;
--
1.5.1.1-dirty
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH] Power supply class and drivers: remove non obligatory return statements
2007-07-15 0:29 ` Jeff Garzik
2007-07-15 1:43 ` [PATCH] pda_power: clean up irq, timer Anton Vorontsov
@ 2007-07-15 1:45 ` Anton Vorontsov
2007-07-15 2:09 ` Jeff Garzik
2007-07-15 4:50 ` [PATCH] pda_power: clean up irq, timer, return usage Andrew Morton
2 siblings, 1 reply; 9+ messages in thread
From: Anton Vorontsov @ 2007-07-15 1:45 UTC (permalink / raw)
To: Jeff Garzik; +Cc: linux-kernel, Andrew Morton, David Woodhouse
Per Jeff Garzik request.
Signed-off-by: Jeff Garzik <jeff@garzik.org>
Signed-off-by: Anton Vorontsov <cbou@mail.ru>
---
drivers/power/apm_power.c | 4 ----
drivers/power/ds2760_battery.c | 7 -------
drivers/power/olpc_battery.c | 1 -
drivers/power/pda_power.c | 5 -----
drivers/power/pmu_battery.c | 2 --
drivers/power/power_supply_core.c | 6 ------
drivers/power/power_supply_leds.c | 8 --------
drivers/power/power_supply_sysfs.c | 2 --
drivers/w1/slaves/w1_ds2760.c | 4 ----
9 files changed, 0 insertions(+), 39 deletions(-)
diff --git a/drivers/power/apm_power.c b/drivers/power/apm_power.c
index 042bd95..39a90a6 100644
--- a/drivers/power/apm_power.c
+++ b/drivers/power/apm_power.c
@@ -48,8 +48,6 @@ static void find_main_battery(void)
}
if (!main_battery)
main_battery = batm;
-
- return;
}
static int calculate_time(int status)
@@ -218,7 +216,6 @@ static void apm_battery_apm_get_power_status(struct apm_power_info *info)
}
up(&power_supply_class->sem);
- return;
}
static int __init apm_battery_init(void)
@@ -232,7 +229,6 @@ static int __init apm_battery_init(void)
static void __exit apm_battery_exit(void)
{
apm_get_power_status = NULL;
- return;
}
module_init(apm_battery_init);
diff --git a/drivers/power/ds2760_battery.c b/drivers/power/ds2760_battery.c
index 00e1ea6..be7021e 100644
--- a/drivers/power/ds2760_battery.c
+++ b/drivers/power/ds2760_battery.c
@@ -254,8 +254,6 @@ static void ds2760_battery_update_status(struct ds2760_device_info *di)
if (di->charge_status != old_charge_status)
power_supply_changed(&di->bat);
-
- return;
}
static void ds2760_battery_work(struct work_struct *work)
@@ -268,8 +266,6 @@ static void ds2760_battery_work(struct work_struct *work)
ds2760_battery_update_status(di);
queue_delayed_work(di->monitor_wqueue, &di->monitor_work, interval);
-
- return;
}
#define to_ds2760_device_info(x) container_of((x), struct ds2760_device_info, \
@@ -283,8 +279,6 @@ static void ds2760_battery_external_power_changed(struct power_supply *psy)
cancel_delayed_work(&di->monitor_work);
queue_delayed_work(di->monitor_wqueue, &di->monitor_work, HZ/10);
-
- return;
}
static int ds2760_battery_get_property(struct power_supply *psy,
@@ -457,7 +451,6 @@ static int __init ds2760_battery_init(void)
static void __exit ds2760_battery_exit(void)
{
platform_driver_unregister(&ds2760_battery_driver);
- return;
}
module_init(ds2760_battery_init);
diff --git a/drivers/power/olpc_battery.c b/drivers/power/olpc_battery.c
index 878684d..c998e68 100644
--- a/drivers/power/olpc_battery.c
+++ b/drivers/power/olpc_battery.c
@@ -341,7 +341,6 @@ static void __exit olpc_bat_exit(void)
power_supply_unregister(&olpc_bat);
power_supply_unregister(&olpc_ac);
platform_device_unregister(bat_pdev);
- return;
}
module_init(olpc_bat_init);
diff --git a/drivers/power/pda_power.c b/drivers/power/pda_power.c
index 8304506..dacbe50 100644
--- a/drivers/power/pda_power.c
+++ b/drivers/power/pda_power.c
@@ -97,8 +97,6 @@ static void update_charger(void)
dev_dbg(dev, "charger off\n");
pdata->set_charge(0);
}
-
- return;
}
static void supply_timer_func(unsigned long power_supply_ptr)
@@ -106,7 +104,6 @@ static void supply_timer_func(unsigned long power_supply_ptr)
void *power_supply = (void *) power_supply_ptr;
power_supply_changed(power_supply);
- return;
}
static void charger_timer_func(unsigned long power_supply_ptr)
@@ -118,7 +115,6 @@ static void charger_timer_func(unsigned long power_supply_ptr)
supply_timer.data = power_supply_ptr;
mod_timer(&supply_timer,
jiffies + msecs_to_jiffies(pdata->wait_for_charger));
- return;
}
static irqreturn_t power_changed_isr(int irq, void *power_supply)
@@ -251,7 +247,6 @@ static int __init pda_power_init(void)
static void __exit pda_power_exit(void)
{
platform_driver_unregister(&pda_power_pdrv);
- return;
}
module_init(pda_power_init);
diff --git a/drivers/power/pmu_battery.c b/drivers/power/pmu_battery.c
index 2fea4af..60a8cf3 100644
--- a/drivers/power/pmu_battery.c
+++ b/drivers/power/pmu_battery.c
@@ -203,8 +203,6 @@ static void __exit pmu_bat_exit(void)
}
power_supply_unregister(&pmu_ac);
platform_device_unregister(bat_pdev);
-
- return;
}
module_init(pmu_bat_init);
diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index e87ea51..a63b75c 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -46,8 +46,6 @@ static void power_supply_changed_work(struct work_struct *work)
power_supply_update_leds(psy);
kobject_uevent(&psy->dev->kobj, KOBJ_CHANGE);
-
- return;
}
void power_supply_changed(struct power_supply *psy)
@@ -55,8 +53,6 @@ void power_supply_changed(struct power_supply *psy)
dev_dbg(psy->dev, "%s\n", __FUNCTION__);
schedule_work(&psy->changed_work);
-
- return;
}
int power_supply_am_i_supplied(struct power_supply *psy)
@@ -129,7 +125,6 @@ void power_supply_unregister(struct power_supply *psy)
power_supply_remove_triggers(psy);
power_supply_remove_attrs(psy);
device_unregister(psy->dev);
- return;
}
static int __init power_supply_class_init(void)
@@ -147,7 +142,6 @@ static int __init power_supply_class_init(void)
static void __exit power_supply_class_exit(void)
{
class_destroy(power_supply_class);
- return;
}
EXPORT_SYMBOL_GPL(power_supply_changed);
diff --git a/drivers/power/power_supply_leds.c b/drivers/power/power_supply_leds.c
index 7232490..7f8f359 100644
--- a/drivers/power/power_supply_leds.c
+++ b/drivers/power/power_supply_leds.c
@@ -40,8 +40,6 @@ static void power_supply_update_bat_leds(struct power_supply *psy)
led_trigger_event(psy->full_trig, LED_OFF);
break;
}
-
- return;
}
static int power_supply_create_bat_triggers(struct power_supply *psy)
@@ -97,7 +95,6 @@ static void power_supply_remove_bat_triggers(struct power_supply *psy)
kfree(psy->full_trig_name);
kfree(psy->charging_trig_name);
kfree(psy->charging_full_trig_name);
- return;
}
/* Generated power specific LEDs triggers. */
@@ -115,8 +112,6 @@ static void power_supply_update_gen_leds(struct power_supply *psy)
led_trigger_event(psy->online_trig, LED_FULL);
else
led_trigger_event(psy->online_trig, LED_OFF);
-
- return;
}
static int power_supply_create_gen_triggers(struct power_supply *psy)
@@ -145,7 +140,6 @@ static void power_supply_remove_gen_triggers(struct power_supply *psy)
{
led_trigger_unregister_simple(psy->online_trig);
kfree(psy->online_trig_name);
- return;
}
/* Choice what triggers to create&update. */
@@ -156,7 +150,6 @@ void power_supply_update_leds(struct power_supply *psy)
power_supply_update_bat_leds(psy);
else
power_supply_update_gen_leds(psy);
- return;
}
int power_supply_create_triggers(struct power_supply *psy)
@@ -172,5 +165,4 @@ void power_supply_remove_triggers(struct power_supply *psy)
power_supply_remove_bat_triggers(psy);
else
power_supply_remove_gen_triggers(psy);
- return;
}
diff --git a/drivers/power/power_supply_sysfs.c b/drivers/power/power_supply_sysfs.c
index c07d425..c7c4574 100644
--- a/drivers/power/power_supply_sysfs.c
+++ b/drivers/power/power_supply_sysfs.c
@@ -176,8 +176,6 @@ void power_supply_remove_attrs(struct power_supply *psy)
for (i = 0; i < psy->num_properties; i++)
device_remove_file(psy->dev,
&power_supply_attrs[psy->properties[i]]);
-
- return;
}
static char *kstruprdup(const char *str, gfp_t gfp)
diff --git a/drivers/w1/slaves/w1_ds2760.c b/drivers/w1/slaves/w1_ds2760.c
index 88a37fb..15e7f9a 100644
--- a/drivers/w1/slaves/w1_ds2760.c
+++ b/drivers/w1/slaves/w1_ds2760.c
@@ -121,8 +121,6 @@ static void release_bat_id(int id)
mutex_lock(&bat_idr_lock);
idr_remove(&bat_idr, id);
mutex_unlock(&bat_idr_lock);
-
- return;
}
static int w1_ds2760_add_slave(struct w1_slave *sl)
@@ -174,8 +172,6 @@ static void w1_ds2760_remove_slave(struct w1_slave *sl)
platform_device_unregister(pdev);
release_bat_id(id);
sysfs_remove_bin_file(&sl->dev.kobj, &w1_ds2760_bin_attr);
-
- return;
}
static struct w1_family_ops w1_ds2760_fops = {
--
1.5.1.1-dirty
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] Power supply class and drivers: remove non obligatory return statements
2007-07-15 1:45 ` [PATCH] Power supply class and drivers: remove non obligatory return statements Anton Vorontsov
@ 2007-07-15 2:09 ` Jeff Garzik
0 siblings, 0 replies; 9+ messages in thread
From: Jeff Garzik @ 2007-07-15 2:09 UTC (permalink / raw)
To: Anton Vorontsov; +Cc: linux-kernel, Andrew Morton, David Woodhouse
Anton Vorontsov wrote:
> Per Jeff Garzik request.
>
> Signed-off-by: Jeff Garzik <jeff@garzik.org>
> Signed-off-by: Anton Vorontsov <cbou@mail.ru>
> ---
> drivers/power/apm_power.c | 4 ----
> drivers/power/ds2760_battery.c | 7 -------
> drivers/power/olpc_battery.c | 1 -
> drivers/power/pda_power.c | 5 -----
> drivers/power/pmu_battery.c | 2 --
> drivers/power/power_supply_core.c | 6 ------
> drivers/power/power_supply_leds.c | 8 --------
> drivers/power/power_supply_sysfs.c | 2 --
> drivers/w1/slaves/w1_ds2760.c | 4 ----
> 9 files changed, 0 insertions(+), 39 deletions(-)
ACK, thanks
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] pda_power: clean up irq, timer, return usage
2007-07-15 0:29 ` Jeff Garzik
2007-07-15 1:43 ` [PATCH] pda_power: clean up irq, timer Anton Vorontsov
2007-07-15 1:45 ` [PATCH] Power supply class and drivers: remove non obligatory return statements Anton Vorontsov
@ 2007-07-15 4:50 ` Andrew Morton
2 siblings, 0 replies; 9+ messages in thread
From: Andrew Morton @ 2007-07-15 4:50 UTC (permalink / raw)
To: Jeff Garzik; +Cc: cbou, LKML
On Sat, 14 Jul 2007 20:29:07 -0400 Jeff Garzik <jeff@garzik.org> wrote:
> Anton Vorontsov wrote:
> > On Sat, Jul 14, 2007 at 07:12:04PM -0400, Jeff Garzik wrote:
> >> Clean up pda_power interrupt handling:
> >
> > Nice, thanks! Just few cosmetic comments.
> >
> >> Prior to this patch, the driver would pass information it needed
> >> to the interrupt handler dev_id pointer, and then prompt forget it
> >> ever did so, recreating that same information after a couple passes
> >> through the timer-based state machine.
> >>
> >> This patch removes the redundant checks by passing the
> >> pda_power_supply[] pointer through the state machine. The current
> >> code passed 'irq' through the state machine, as an index to recreate
> >> the pointer, when we could more simply pass around the pointer itself.
> >>
> >> Additionally, bogus "return;" statements were removed.
> >
> > My preference is to use "return;" statements in functions returning
> > void, even if functions is very small. It's just sugar for my eyes,
> > to really see exit points. . Without returns I've feeling that
> > something is missing there. Yes, my oddity.
> >
> > Plus, so far CodingStyle does not say anything about non obligatory
> > return statements.
> >
> > These should be "fixed" too, though:
> > ~/linux-2.6$ grep -h "return;" -A1 -r drivers/ arch/ | grep "^}$" | \
> > wc -l
> > 1354
> >
> > Obviously, drivers/ata is almost pure (3). ;-)
> >
> > Either way, I prefer to leave alone these "return;"s, until CodingStyle
> > permits them.
>
> CodingStyle is not the end-all of rulebooks. See repeated messages by
> Linus, me, and others on the subject. CodingStyle intentionally does
> not list a rule for every possible C code incarnation.
Yup, the `return' is a waste of space.
> > I'll send patch shortly.
> >
> >> + void *power_supply = (void *) power_supply_ptr;
> > ^
> > I guess common practice is not to put space here.
>
> incorrect. a space goes there, as I put it.
>
Nope, the space is a waste of space.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] MAINTAINERS: Add maintainers for power supply subsystem and drivers (was [PATCH] pda_power: clean up irq, timer, return usage)
2007-07-15 1:02 ` [PATCH] MAINTAINERS: Add maintainers for power supply subsystem and drivers (was [PATCH] pda_power: clean up irq, timer, return usage) Anton Vorontsov
@ 2007-07-15 7:54 ` David Woodhouse
0 siblings, 0 replies; 9+ messages in thread
From: David Woodhouse @ 2007-07-15 7:54 UTC (permalink / raw)
To: cbou; +Cc: Andrew Morton, LKML, Jeff Garzik
On Sun, 2007-07-15 at 05:02 +0400, Anton Vorontsov wrote:
> On Sat, Jul 14, 2007 at 07:12:04PM -0400, Jeff Garzik wrote:
> > P.S. Where are the MAINTAINERS entries for this driver, and
> > drivers/power in general?
>
> Collecting acks. David?
>
> From: Anton Vorontsov <cbou@mail.ru>
> Subject: [PATCH] MAINTAINERS: Add maintainers for power supply subsystem and drivers
>
> Signed-off-by: Anton Vorontsov <cbou@mail.ru>
Looks good to me; except...
> +T: git git.infradead.org/~cbou/battery2-2.6.git
> +T: git git.infradead.org/~dwmw2/battery-2.6.git
Let's just use a single tree. I created
git://git.infradead.org/battery-2.6.git which is group-writable.
(it's in /srv/git on bombadil)
--
dwmw2
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2007-07-15 7:55 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-07-14 23:12 [PATCH] pda_power: clean up irq, timer, return usage Jeff Garzik
2007-07-15 0:15 ` Anton Vorontsov
2007-07-15 0:29 ` Jeff Garzik
2007-07-15 1:43 ` [PATCH] pda_power: clean up irq, timer Anton Vorontsov
2007-07-15 1:45 ` [PATCH] Power supply class and drivers: remove non obligatory return statements Anton Vorontsov
2007-07-15 2:09 ` Jeff Garzik
2007-07-15 4:50 ` [PATCH] pda_power: clean up irq, timer, return usage Andrew Morton
2007-07-15 1:02 ` [PATCH] MAINTAINERS: Add maintainers for power supply subsystem and drivers (was [PATCH] pda_power: clean up irq, timer, return usage) Anton Vorontsov
2007-07-15 7:54 ` David Woodhouse
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox