2021-03-23 13:02:40

by Xiaofei Tan

[permalink] [raw]
Subject: [PATCH] ACPI: AC: fix some errors and warnings reported by checkpatch.pl

Fix some errors and warnings reported by checkpatch.pl, including
following five types:
ERROR: "foo * bar" should be "foo *bar"
ERROR: code indent should use tabs where possible
WARNING: Block comments use a trailing */ on a separate line
WARNING: braces {} are not necessary for single statement blocks
WARNING: void function return statements are not generally useful

Signed-off-by: Xiaofei Tan <[email protected]>
---
drivers/acpi/ac.c | 32 +++++++++++++-------------------
1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index b86ee6e..07987854 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -78,17 +78,16 @@ static struct acpi_driver acpi_ac_driver = {
struct acpi_ac {
struct power_supply *charger;
struct power_supply_desc charger_desc;
- struct acpi_device * device;
+ struct acpi_device *device;
unsigned long long state;
struct notifier_block battery_nb;
};

#define to_acpi_ac(x) power_supply_get_drvdata(x)

-/* --------------------------------------------------------------------------
- AC Adapter Management
- -------------------------------------------------------------------------- */
-
+/*
+ * AC Adapter Management
+ */
static int acpi_ac_get_state(struct acpi_ac *ac)
{
acpi_status status = AE_OK;
@@ -109,9 +108,9 @@ static int acpi_ac_get_state(struct acpi_ac *ac)
return 0;
}

-/* --------------------------------------------------------------------------
- sysfs I/F
- -------------------------------------------------------------------------- */
+/*
+ * sysfs I/F
+ */
static int get_ac_property(struct power_supply *psy,
enum power_supply_property psp,
union power_supply_propval *val)
@@ -138,10 +137,9 @@ static enum power_supply_property ac_props[] = {
POWER_SUPPLY_PROP_ONLINE,
};

-/* --------------------------------------------------------------------------
- Driver Model
- -------------------------------------------------------------------------- */
-
+/*
+ * Driver Model
+ */
static void acpi_ac_notify(struct acpi_device *device, u32 event)
{
struct acpi_ac *ac = acpi_driver_data(device);
@@ -174,8 +172,6 @@ static void acpi_ac_notify(struct acpi_device *device, u32 event)
acpi_notifier_call_chain(device, event, (u32) ac->state);
kobject_uevent(&ac->charger->dev.kobj, KOBJ_CHANGE);
}
-
- return;
}

static int acpi_ac_battery_notify(struct notifier_block *nb,
@@ -282,9 +278,8 @@ static int acpi_ac_add(struct acpi_device *device)
ac->battery_nb.notifier_call = acpi_ac_battery_notify;
register_acpi_notifier(&ac->battery_nb);
end:
- if (result) {
+ if (result)
kfree(ac);
- }

return result;
}
@@ -293,7 +288,7 @@ static int acpi_ac_add(struct acpi_device *device)
static int acpi_ac_resume(struct device *dev)
{
struct acpi_ac *ac;
- unsigned old_state;
+ unsigned int old_state;

if (!dev)
return -EINVAL;
@@ -352,9 +347,8 @@ static int __init acpi_ac_init(void)
}

result = acpi_bus_register_driver(&acpi_ac_driver);
- if (result < 0) {
+ if (result < 0)
return -ENODEV;
- }

return 0;
}
--
2.8.1


2021-03-25 03:14:40

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] ACPI: AC: fix some errors and warnings reported by checkpatch.pl

On Tue, Mar 23, 2021 at 2:01 PM Xiaofei Tan <[email protected]> wrote:
>
> Fix some errors and warnings reported by checkpatch.pl, including
> following five types:

Well, they are coding style issues rather than errors.

> ERROR: "foo * bar" should be "foo *bar"
> ERROR: code indent should use tabs where possible
> WARNING: Block comments use a trailing */ on a separate line
> WARNING: braces {} are not necessary for single statement blocks
> WARNING: void function return statements are not generally useful
>
> Signed-off-by: Xiaofei Tan <[email protected]>
> ---
> drivers/acpi/ac.c | 32 +++++++++++++-------------------
> 1 file changed, 13 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
> index b86ee6e..07987854 100644
> --- a/drivers/acpi/ac.c
> +++ b/drivers/acpi/ac.c
> @@ -78,17 +78,16 @@ static struct acpi_driver acpi_ac_driver = {
> struct acpi_ac {
> struct power_supply *charger;
> struct power_supply_desc charger_desc;
> - struct acpi_device * device;
> + struct acpi_device *device;
> unsigned long long state;
> struct notifier_block battery_nb;
> };
>
> #define to_acpi_ac(x) power_supply_get_drvdata(x)
>
> -/* --------------------------------------------------------------------------
> - AC Adapter Management
> - -------------------------------------------------------------------------- */
> -
> +/*
> + * AC Adapter Management
> + */

Please use the /* ... */ (one-line) comment format here and below for
comments that aren't longer than one line.

> static int acpi_ac_get_state(struct acpi_ac *ac)
> {
> acpi_status status = AE_OK;
> @@ -109,9 +108,9 @@ static int acpi_ac_get_state(struct acpi_ac *ac)
> return 0;
> }
>
> -/* --------------------------------------------------------------------------
> - sysfs I/F
> - -------------------------------------------------------------------------- */
> +/*
> + * sysfs I/F
> + */
> static int get_ac_property(struct power_supply *psy,
> enum power_supply_property psp,
> union power_supply_propval *val)
> @@ -138,10 +137,9 @@ static enum power_supply_property ac_props[] = {
> POWER_SUPPLY_PROP_ONLINE,
> };
>
> -/* --------------------------------------------------------------------------
> - Driver Model
> - -------------------------------------------------------------------------- */
> -
> +/*
> + * Driver Model
> + */
> static void acpi_ac_notify(struct acpi_device *device, u32 event)
> {
> struct acpi_ac *ac = acpi_driver_data(device);
> @@ -174,8 +172,6 @@ static void acpi_ac_notify(struct acpi_device *device, u32 event)
> acpi_notifier_call_chain(device, event, (u32) ac->state);
> kobject_uevent(&ac->charger->dev.kobj, KOBJ_CHANGE);
> }
> -
> - return;
> }
>
> static int acpi_ac_battery_notify(struct notifier_block *nb,
> @@ -282,9 +278,8 @@ static int acpi_ac_add(struct acpi_device *device)
> ac->battery_nb.notifier_call = acpi_ac_battery_notify;
> register_acpi_notifier(&ac->battery_nb);
> end:
> - if (result) {
> + if (result)
> kfree(ac);
> - }
>
> return result;
> }
> @@ -293,7 +288,7 @@ static int acpi_ac_add(struct acpi_device *device)
> static int acpi_ac_resume(struct device *dev)
> {
> struct acpi_ac *ac;
> - unsigned old_state;
> + unsigned int old_state;
>
> if (!dev)
> return -EINVAL;
> @@ -352,9 +347,8 @@ static int __init acpi_ac_init(void)
> }
>
> result = acpi_bus_register_driver(&acpi_ac_driver);
> - if (result < 0) {
> + if (result < 0)
> return -ENODEV;
> - }
>
> return 0;
> }
> --

2021-03-25 03:42:51

by Xiaofei Tan

[permalink] [raw]
Subject: Re: [PATCH] ACPI: AC: fix some errors and warnings reported by checkpatch.pl

Hi Rafael,
BTW, there is still one warning left reported by checkpatch.pl in this
file. The cause of the warning is that it contains an special character
'$'. Does it have any special purpose? If no, i can fix it in the new
version, too. thanks.

WARNING: CVS style keyword markers, these will _not_ be updated
#3: FILE: drivers/acpi/ac.c:3:
+ * acpi_ac.c - ACPI AC Adapter Driver ($Revision: 27 $)

On 2021/3/25 9:26, Xiaofei Tan wrote:
> Hi Rafael,
>
> On 2021/3/24 23:57, Rafael J. Wysocki wrote:
>> On Tue, Mar 23, 2021 at 2:01 PM Xiaofei Tan <[email protected]>
>> wrote:
>>>
>>> Fix some errors and warnings reported by checkpatch.pl, including
>>> following five types:
>>
>> Well, they are coding style issues rather than errors.
>>
>
> Right, i could change the description.
>
>>> ERROR: "foo * bar" should be "foo *bar"
>>> ERROR: code indent should use tabs where possible
>>> WARNING: Block comments use a trailing */ on a separate line
>>> WARNING: braces {} are not necessary for single statement blocks
>>> WARNING: void function return statements are not generally useful
>>>
>>> Signed-off-by: Xiaofei Tan <[email protected]>
>>> ---
>>> drivers/acpi/ac.c | 32 +++++++++++++-------------------
>>> 1 file changed, 13 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
>>> index b86ee6e..07987854 100644
>>> --- a/drivers/acpi/ac.c
>>> +++ b/drivers/acpi/ac.c
>>> @@ -78,17 +78,16 @@ static struct acpi_driver acpi_ac_driver = {
>>> struct acpi_ac {
>>> struct power_supply *charger;
>>> struct power_supply_desc charger_desc;
>>> - struct acpi_device * device;
>>> + struct acpi_device *device;
>>> unsigned long long state;
>>> struct notifier_block battery_nb;
>>> };
>>>
>>> #define to_acpi_ac(x) power_supply_get_drvdata(x)
>>>
>>> -/*
>>> --------------------------------------------------------------------------
>>>
>>> - AC Adapter Management
>>> -
>>> --------------------------------------------------------------------------
>>> */
>>> -
>>> +/*
>>> + * AC Adapter Management
>>> + */
>>
>> Please use the /* ... */ (one-line) comment format here and below for
>> comments that aren't longer than one line.
>>
>
> Sure.
>
>>> static int acpi_ac_get_state(struct acpi_ac *ac)
>>> {
>>> acpi_status status = AE_OK;
>>> @@ -109,9 +108,9 @@ static int acpi_ac_get_state(struct acpi_ac *ac)
>>> return 0;
>>> }
>>>
>>> -/*
>>> --------------------------------------------------------------------------
>>>
>>> - sysfs I/F
>>> -
>>> --------------------------------------------------------------------------
>>> */
>>> +/*
>>> + * sysfs I/F
>>> + */
>>> static int get_ac_property(struct power_supply *psy,
>>> enum power_supply_property psp,
>>> union power_supply_propval *val)
>>> @@ -138,10 +137,9 @@ static enum power_supply_property ac_props[] = {
>>> POWER_SUPPLY_PROP_ONLINE,
>>> };
>>>
>>> -/*
>>> --------------------------------------------------------------------------
>>>
>>> - Driver Model
>>> -
>>> --------------------------------------------------------------------------
>>> */
>>> -
>>> +/*
>>> + * Driver Model
>>> + */
>>> static void acpi_ac_notify(struct acpi_device *device, u32 event)
>>> {
>>> struct acpi_ac *ac = acpi_driver_data(device);
>>> @@ -174,8 +172,6 @@ static void acpi_ac_notify(struct acpi_device
>>> *device, u32 event)
>>> acpi_notifier_call_chain(device, event, (u32)
>>> ac->state);
>>> kobject_uevent(&ac->charger->dev.kobj, KOBJ_CHANGE);
>>> }
>>> -
>>> - return;
>>> }
>>>
>>> static int acpi_ac_battery_notify(struct notifier_block *nb,
>>> @@ -282,9 +278,8 @@ static int acpi_ac_add(struct acpi_device *device)
>>> ac->battery_nb.notifier_call = acpi_ac_battery_notify;
>>> register_acpi_notifier(&ac->battery_nb);
>>> end:
>>> - if (result) {
>>> + if (result)
>>> kfree(ac);
>>> - }
>>>
>>> return result;
>>> }
>>> @@ -293,7 +288,7 @@ static int acpi_ac_add(struct acpi_device *device)
>>> static int acpi_ac_resume(struct device *dev)
>>> {
>>> struct acpi_ac *ac;
>>> - unsigned old_state;
>>> + unsigned int old_state;
>>>
>>> if (!dev)
>>> return -EINVAL;
>>> @@ -352,9 +347,8 @@ static int __init acpi_ac_init(void)
>>> }
>>>
>>> result = acpi_bus_register_driver(&acpi_ac_driver);
>>> - if (result < 0) {
>>> + if (result < 0)
>>> return -ENODEV;
>>> - }
>>>
>>> return 0;
>>> }
>>> --
>>
>> .
>>

2021-03-25 03:43:01

by Xiaofei Tan

[permalink] [raw]
Subject: Re: [PATCH] ACPI: AC: fix some errors and warnings reported by checkpatch.pl

Hi Rafael,

On 2021/3/24 23:57, Rafael J. Wysocki wrote:
> On Tue, Mar 23, 2021 at 2:01 PM Xiaofei Tan <[email protected]> wrote:
>>
>> Fix some errors and warnings reported by checkpatch.pl, including
>> following five types:
>
> Well, they are coding style issues rather than errors.
>

Right, i could change the description.

>> ERROR: "foo * bar" should be "foo *bar"
>> ERROR: code indent should use tabs where possible
>> WARNING: Block comments use a trailing */ on a separate line
>> WARNING: braces {} are not necessary for single statement blocks
>> WARNING: void function return statements are not generally useful
>>
>> Signed-off-by: Xiaofei Tan <[email protected]>
>> ---
>> drivers/acpi/ac.c | 32 +++++++++++++-------------------
>> 1 file changed, 13 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
>> index b86ee6e..07987854 100644
>> --- a/drivers/acpi/ac.c
>> +++ b/drivers/acpi/ac.c
>> @@ -78,17 +78,16 @@ static struct acpi_driver acpi_ac_driver = {
>> struct acpi_ac {
>> struct power_supply *charger;
>> struct power_supply_desc charger_desc;
>> - struct acpi_device * device;
>> + struct acpi_device *device;
>> unsigned long long state;
>> struct notifier_block battery_nb;
>> };
>>
>> #define to_acpi_ac(x) power_supply_get_drvdata(x)
>>
>> -/* --------------------------------------------------------------------------
>> - AC Adapter Management
>> - -------------------------------------------------------------------------- */
>> -
>> +/*
>> + * AC Adapter Management
>> + */
>
> Please use the /* ... */ (one-line) comment format here and below for
> comments that aren't longer than one line.
>

Sure.

>> static int acpi_ac_get_state(struct acpi_ac *ac)
>> {
>> acpi_status status = AE_OK;
>> @@ -109,9 +108,9 @@ static int acpi_ac_get_state(struct acpi_ac *ac)
>> return 0;
>> }
>>
>> -/* --------------------------------------------------------------------------
>> - sysfs I/F
>> - -------------------------------------------------------------------------- */
>> +/*
>> + * sysfs I/F
>> + */
>> static int get_ac_property(struct power_supply *psy,
>> enum power_supply_property psp,
>> union power_supply_propval *val)
>> @@ -138,10 +137,9 @@ static enum power_supply_property ac_props[] = {
>> POWER_SUPPLY_PROP_ONLINE,
>> };
>>
>> -/* --------------------------------------------------------------------------
>> - Driver Model
>> - -------------------------------------------------------------------------- */
>> -
>> +/*
>> + * Driver Model
>> + */
>> static void acpi_ac_notify(struct acpi_device *device, u32 event)
>> {
>> struct acpi_ac *ac = acpi_driver_data(device);
>> @@ -174,8 +172,6 @@ static void acpi_ac_notify(struct acpi_device *device, u32 event)
>> acpi_notifier_call_chain(device, event, (u32) ac->state);
>> kobject_uevent(&ac->charger->dev.kobj, KOBJ_CHANGE);
>> }
>> -
>> - return;
>> }
>>
>> static int acpi_ac_battery_notify(struct notifier_block *nb,
>> @@ -282,9 +278,8 @@ static int acpi_ac_add(struct acpi_device *device)
>> ac->battery_nb.notifier_call = acpi_ac_battery_notify;
>> register_acpi_notifier(&ac->battery_nb);
>> end:
>> - if (result) {
>> + if (result)
>> kfree(ac);
>> - }
>>
>> return result;
>> }
>> @@ -293,7 +288,7 @@ static int acpi_ac_add(struct acpi_device *device)
>> static int acpi_ac_resume(struct device *dev)
>> {
>> struct acpi_ac *ac;
>> - unsigned old_state;
>> + unsigned int old_state;
>>
>> if (!dev)
>> return -EINVAL;
>> @@ -352,9 +347,8 @@ static int __init acpi_ac_init(void)
>> }
>>
>> result = acpi_bus_register_driver(&acpi_ac_driver);
>> - if (result < 0) {
>> + if (result < 0)
>> return -ENODEV;
>> - }
>>
>> return 0;
>> }
>> --
>
> .
>