[PATCH v2 00/15] acpi: fix some coding style issues

[PATCH v2 00/15] acpi: fix some coding style issues

[Xiaofei Tan]

Fix some coding style issues reported by checkpatch.pl.

Differences from v1 to v2:
- Add subsystem and module name in the name of patch 05/15.
- Change to use more proper module name for some patch names.

Xiaofei Tan (15):
  ACPI: APD: fix a block comment align issue
  ACPI: processor: fix some coding style issues
  ACPI: debug: fix some coding style issues
  ACPI: table: replace __attribute__((packed)) by __packed
  ACPI: ipmi: remove useless return statement for void function
  ACPI: LPSS: fix some coding style issues
  ACPI: memhotplug: fix a coding style issue
  ACPI: acpi_pad: fix a coding style issue
  ACPI: battery: fix some coding style issues
  ACPI: button: fix some coding style issues
  ACPI: CPPC: fix some coding style issues
  ACPI: custom_method: fix a coding style issue
  ACPI: PM: fix some coding style issues
  ACPI: sysfs: fix some coding style issues
  ACPI: dock: fix some coding style issues

 drivers/acpi/acpi_apd.c        |  8 ++---
 drivers/acpi/acpi_dbg.c        | 40 +++++++++++-------------
 drivers/acpi/acpi_fpdt.c       |  6 ++--
 drivers/acpi/acpi_ipmi.c       |  1 -
 drivers/acpi/acpi_lpss.c       |  4 ++-
 drivers/acpi/acpi_memhotplug.c |  2 +-
 drivers/acpi/acpi_pad.c        |  4 +++
 drivers/acpi/acpi_processor.c  | 18 +++--------
 drivers/acpi/battery.c         | 64 +++++++++++++++++++++----------------
 drivers/acpi/button.c          | 10 +++---
 drivers/acpi/cppc_acpi.c       | 71 +++++++++++++++++++++---------------------
 drivers/acpi/custom_method.c   |  2 +-
 drivers/acpi/device_pm.c       |  3 ++
 drivers/acpi/device_sysfs.c    | 15 ++++++---
 drivers/acpi/dock.c            |  7 +++--
 15 files changed, 138 insertions(+), 117 deletions(-)

-- 
2.8.1

[PATCH v2 01/15] ACPI: APD: fix a block comment align issue

[Xiaofei Tan]

Fix the following coding style issue reported by checkpatch.pl.
WARNING: Block comments should align the * on each line
+/**
+* Create platform device during acpi scan attach handle.

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
---
 drivers/acpi/acpi_apd.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/acpi_apd.c b/drivers/acpi/acpi_apd.c
index 39359ce..0ec5b3f 100644
--- a/drivers/acpi/acpi_apd.c
+++ b/drivers/acpi/acpi_apd.c
@@ -176,10 +176,10 @@ static const struct apd_device_desc hip08_spi_desc = {
 
 #endif
 
-/**
-* Create platform device during acpi scan attach handle.
-* Return value > 0 on success of creating device.
-*/
+/*
+ * Create platform device during acpi scan attach handle.
+ * Return value > 0 on success of creating device.
+ */
 static int acpi_apd_create_device(struct acpi_device *adev,
 				   const struct acpi_device_id *id)
 {
-- 
2.8.1

[PATCH v2 02/15] ACPI: processor: fix some coding style issues

[Xiaofei Tan]

Fix some coding style issues reported by checkpatch.pl, including
following types:

ERROR: code indent should use tabs where possible
WARNING: Block comments use a trailing */ on a separate line
WARNING: Missing a blank line after declarations
WARNING: labels should not be indented

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
---
 drivers/acpi/acpi_processor.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index fc89f3a..2d5bd2a 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -25,10 +25,7 @@
 DEFINE_PER_CPU(struct acpi_processor *, processors);
 EXPORT_PER_CPU_SYMBOL(processors);
 
-/* --------------------------------------------------------------------------
-                                Errata Handling
-   -------------------------------------------------------------------------- */
-
+/* Errata Handling */
 struct acpi_processor_errata errata __read_mostly;
 EXPORT_SYMBOL_GPL(errata);
 
@@ -151,10 +148,7 @@ static int acpi_processor_errata(void)
 	return result;
 }
 
-/* --------------------------------------------------------------------------
-                                Initialization
-   -------------------------------------------------------------------------- */
-
+/* Initialization */
 #ifdef CONFIG_ACPI_HOTPLUG_CPU
 int __weak acpi_map_cpu(acpi_handle handle,
 		phys_cpuid_t physid, u32 acpi_id, int *pcpu)
@@ -306,6 +300,7 @@ static int acpi_processor_get_info(struct acpi_device *device)
 	 */
 	if (invalid_logical_cpuid(pr->id) || !cpu_present(pr->id)) {
 		int ret = acpi_processor_hotadd_init(pr);
+
 		if (ret)
 			return ret;
 	}
@@ -431,10 +426,7 @@ static int acpi_processor_add(struct acpi_device *device,
 }
 
 #ifdef CONFIG_ACPI_HOTPLUG_CPU
-/* --------------------------------------------------------------------------
-                                    Removal
-   -------------------------------------------------------------------------- */
-
+/* Removal */
 static void acpi_processor_remove(struct acpi_device *device)
 {
 	struct acpi_processor *pr;
@@ -892,7 +884,7 @@ int acpi_processor_evaluate_cst(acpi_handle handle, u32 cpu,
 
 	info->count = last_index;
 
-      end:
+end:
 	kfree(buffer.pointer);
 
 	return ret;
-- 
2.8.1

[PATCH v2 03/15] ACPI: debug: fix some coding style issues

[Xiaofei Tan]

Fix some coding style issues reported by checkpatch.pl, including
following types:

WARNING: space prohibited between function name and open parenthesis
WARNING: else is not generally useful after a break or return

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
---
 drivers/acpi/acpi_dbg.c | 40 +++++++++++++++++++---------------------
 1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/drivers/acpi/acpi_dbg.c b/drivers/acpi/acpi_dbg.c
index d50261d..e641bc1 100644
--- a/drivers/acpi/acpi_dbg.c
+++ b/drivers/acpi/acpi_dbg.c
@@ -21,7 +21,7 @@
 #include <linux/acpi.h>
 #include "internal.h"
 
-#define ACPI_AML_BUF_ALIGN	(sizeof (acpi_size))
+#define ACPI_AML_BUF_ALIGN	(sizeof(acpi_size))
 #define ACPI_AML_BUF_SIZE	PAGE_SIZE
 
 #define circ_count(circ) \
@@ -613,16 +613,15 @@ static ssize_t acpi_aml_read(struct file *file, char __user *buf,
 		if (ret == -EAGAIN) {
 			if (file->f_flags & O_NONBLOCK)
 				break;
-			else {
-				ret = wait_event_interruptible(acpi_aml_io.wait,
-					acpi_aml_user_readable());
-				/*
-				 * We need to retry when the condition
-				 * becomes true.
-				 */
-				if (ret == 0)
-					goto again;
-			}
+
+			ret = wait_event_interruptible(acpi_aml_io.wait,
+				acpi_aml_user_readable());
+			/*
+			 * We need to retry when the condition
+			 * becomes true.
+			 */
+			if (ret == 0)
+				goto again;
 		}
 		if (ret < 0) {
 			if (!acpi_aml_running())
@@ -683,16 +682,15 @@ static ssize_t acpi_aml_write(struct file *file, const char __user *buf,
 		if (ret == -EAGAIN) {
 			if (file->f_flags & O_NONBLOCK)
 				break;
-			else {
-				ret = wait_event_interruptible(acpi_aml_io.wait,
-					acpi_aml_user_writable());
-				/*
-				 * We need to retry when the condition
-				 * becomes true.
-				 */
-				if (ret == 0)
-					goto again;
-			}
+
+			ret = wait_event_interruptible(acpi_aml_io.wait,
+				acpi_aml_user_writable());
+			/*
+			 * We need to retry when the condition
+			 * becomes true.
+			 */
+			if (ret == 0)
+				goto again;
 		}
 		if (ret < 0) {
 			if (!acpi_aml_running())
-- 
2.8.1

[PATCH v2 04/15] ACPI: table: replace __attribute__((packed)) by __packed

[Xiaofei Tan]

Replace __attribute__((packed)) by __packed following the
advice of checkpatch.pl.

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
---
 drivers/acpi/acpi_fpdt.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/acpi_fpdt.c b/drivers/acpi/acpi_fpdt.c
index a89a806..690a88a 100644
--- a/drivers/acpi/acpi_fpdt.c
+++ b/drivers/acpi/acpi_fpdt.c
@@ -53,7 +53,7 @@ struct resume_performance_record {
 	u32 resume_count;
 	u64 resume_prev;
 	u64 resume_avg;
-} __attribute__((packed));
+} __packed;
 
 struct boot_performance_record {
 	struct fpdt_record_header header;
@@ -63,13 +63,13 @@ struct boot_performance_record {
 	u64 bootloader_launch;
 	u64 exitbootservice_start;
 	u64 exitbootservice_end;
-} __attribute__((packed));
+} __packed;
 
 struct suspend_performance_record {
 	struct fpdt_record_header header;
 	u64 suspend_start;
 	u64 suspend_end;
-} __attribute__((packed));
+} __packed;
 
 
 static struct resume_performance_record *record_resume;
-- 
2.8.1

RE: [PATCH v2 04/15] ACPI: table: replace __attribute__((packed)) by __packed

[David Laight]

From: Xiaofei Tan
> Sent: 27 March 2021 07:46
> 
> Replace __attribute__((packed)) by __packed following the
> advice of checkpatch.pl.
> 
> Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
> ---
>  drivers/acpi/acpi_fpdt.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/acpi/acpi_fpdt.c b/drivers/acpi/acpi_fpdt.c
> index a89a806..690a88a 100644
> --- a/drivers/acpi/acpi_fpdt.c
> +++ b/drivers/acpi/acpi_fpdt.c
> @@ -53,7 +53,7 @@ struct resume_performance_record {
>  	u32 resume_count;
>  	u64 resume_prev;
>  	u64 resume_avg;
> -} __attribute__((packed));
> +} __packed;
> 
>  struct boot_performance_record {
>  	struct fpdt_record_header header;
> @@ -63,13 +63,13 @@ struct boot_performance_record {
>  	u64 bootloader_launch;
>  	u64 exitbootservice_start;
>  	u64 exitbootservice_end;
> -} __attribute__((packed));
> +} __packed;
> 
>  struct suspend_performance_record {
>  	struct fpdt_record_header header;
>  	u64 suspend_start;
>  	u64 suspend_end;
> -} __attribute__((packed));
> +} __packed;

My standard question about 'packed' is whether it is actually needed.
It should only be used if the structures might be misaligned in memory.
If the only problem is that a 64bit item needs to be 32bit aligned
then a suitable type should be used for those specific fields.

Those all look very dubious - the standard header isn't packed
so everything must eb assumed to be at least 32bit aligned.

There are also other sub-structures that contain 64bit values.
These don't contain padding - but that requires 64bit alignement.

The only problematic structure is the last one - which would have
a 32bit pad after the header.
Is this even right given than there are explicit alignment pads
in some of the other structures.

If 64bit alignment isn't guaranteed then a '64bit aligned to 32bit'
type should be used for the u64 fields.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH v2 04/15] ACPI: table: replace __attribute__((packed)) by __packed

[Xiaofei Tan]

Hi David,

On 2021/3/29 18:09, David Laight wrote:
> From: Xiaofei Tan
>> Sent: 27 March 2021 07:46
>>
>> Replace __attribute__((packed)) by __packed following the
>> advice of checkpatch.pl.
>>
>> Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
>> ---
>>  drivers/acpi/acpi_fpdt.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/acpi/acpi_fpdt.c b/drivers/acpi/acpi_fpdt.c
>> index a89a806..690a88a 100644
>> --- a/drivers/acpi/acpi_fpdt.c
>> +++ b/drivers/acpi/acpi_fpdt.c
>> @@ -53,7 +53,7 @@ struct resume_performance_record {
>>  	u32 resume_count;
>>  	u64 resume_prev;
>>  	u64 resume_avg;
>> -} __attribute__((packed));
>> +} __packed;
>>
>>  struct boot_performance_record {
>>  	struct fpdt_record_header header;
>> @@ -63,13 +63,13 @@ struct boot_performance_record {
>>  	u64 bootloader_launch;
>>  	u64 exitbootservice_start;
>>  	u64 exitbootservice_end;
>> -} __attribute__((packed));
>> +} __packed;
>>
>>  struct suspend_performance_record {
>>  	struct fpdt_record_header header;
>>  	u64 suspend_start;
>>  	u64 suspend_end;
>> -} __attribute__((packed));
>> +} __packed;
>
> My standard question about 'packed' is whether it is actually needed.
> It should only be used if the structures might be misaligned in memory.
> If the only problem is that a 64bit item needs to be 32bit aligned
> then a suitable type should be used for those specific fields.
>
> Those all look very dubious - the standard header isn't packed
> so everything must eb assumed to be at least 32bit aligned.
>
> There are also other sub-structures that contain 64bit values.
> These don't contain padding - but that requires 64bit alignement.
>
> The only problematic structure is the last one - which would have
> a 32bit pad after the header.
> Is this even right given than there are explicit alignment pads
> in some of the other structures.
>
> If 64bit alignment isn't guaranteed then a '64bit aligned to 32bit'
> type should be used for the u64 fields.
>

Yes, some of them has been aligned already, then nothing changed when 
add this "packed ". Maybe the purpose of the original author is for 
extension, and can tell others that this struct need be packed.

> 	David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>
>
> .
>

Re: [PATCH v2 04/15] ACPI: table: replace __attribute__((packed)) by __packed

[Zhang Rui]

On Tue, 2021-03-30 at 10:23 +0800, Xiaofei Tan wrote:
> Hi David,
> 
> On 2021/3/29 18:09, David Laight wrote:
> > From: Xiaofei Tan
> > > Sent: 27 March 2021 07:46
> > > 
> > > Replace __attribute__((packed)) by __packed following the
> > > advice of checkpatch.pl.
> > > 
> > > Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
> > > ---
> > >  drivers/acpi/acpi_fpdt.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/acpi/acpi_fpdt.c b/drivers/acpi/acpi_fpdt.c
> > > index a89a806..690a88a 100644
> > > --- a/drivers/acpi/acpi_fpdt.c
> > > +++ b/drivers/acpi/acpi_fpdt.c
> > > @@ -53,7 +53,7 @@ struct resume_performance_record {
> > >  	u32 resume_count;
> > >  	u64 resume_prev;
> > >  	u64 resume_avg;
> > > -} __attribute__((packed));
> > > +} __packed;
> > > 
> > >  struct boot_performance_record {
> > >  	struct fpdt_record_header header;
> > > @@ -63,13 +63,13 @@ struct boot_performance_record {
> > >  	u64 bootloader_launch;
> > >  	u64 exitbootservice_start;
> > >  	u64 exitbootservice_end;
> > > -} __attribute__((packed));
> > > +} __packed;
> > > 
> > >  struct suspend_performance_record {
> > >  	struct fpdt_record_header header;
> > >  	u64 suspend_start;
> > >  	u64 suspend_end;
> > > -} __attribute__((packed));
> > > +} __packed;
> > 
> > My standard question about 'packed' is whether it is actually
> > needed.
> > It should only be used if the structures might be misaligned in
> > memory.
> > If the only problem is that a 64bit item needs to be 32bit aligned
> > then a suitable type should be used for those specific fields.
> > 
> > Those all look very dubious - the standard header isn't packed
> > so everything must eb assumed to be at least 32bit aligned.
> > 
> > There are also other sub-structures that contain 64bit values.
> > These don't contain padding - but that requires 64bit alignement.
> > 
> > The only problematic structure is the last one - which would have
> > a 32bit pad after the header.
> > Is this even right given than there are explicit alignment pads
> > in some of the other structures.
> > 
> > If 64bit alignment isn't guaranteed then a '64bit aligned to 32bit'
> > type should be used for the u64 fields.
> > 
> 
> Yes, some of them has been aligned already, then nothing changed
> when 
> add this "packed ". Maybe the purpose of the original author is for 
> extension, and can tell others that this struct need be packed.
> 

The patch is upstreamed recently but it was made long time ago.
I think the original problem is that one of the address, probably the
suspend_performance record, is not 64bit aligned, thus we can not read
the proper content of suspend_start and suspend_end, mapped from
physical memory.

I will try to find a machine to reproduce the problem with all
__attribute__((packed)) removed to double confirm this.

thanks,
rui
> > 	David
> > 
> > -
> > Registered Address Lakeside, Bramley Road, Mount Farm, Milton
> > Keynes, MK1 1PT, UK
> > Registration No: 1397386 (Wales)
> > 
> > 
> > .
> > 
> 
> 

Re: [PATCH v2 04/15] ACPI: table: replace __attribute__((packed)) by __packed

[Zhang Rui]

On Tue, 2021-03-30 at 15:31 +0800, Zhang Rui wrote:
> On Tue, 2021-03-30 at 10:23 +0800, Xiaofei Tan wrote:
> > Hi David,
> > 
> > On 2021/3/29 18:09, David Laight wrote:
> > > From: Xiaofei Tan
> > > > Sent: 27 March 2021 07:46
> > > > 
> > > > Replace __attribute__((packed)) by __packed following the
> > > > advice of checkpatch.pl.
> > > > 
> > > > Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
> > > > ---
> > > >  drivers/acpi/acpi_fpdt.c | 6 +++---
> > > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/acpi/acpi_fpdt.c
> > > > b/drivers/acpi/acpi_fpdt.c
> > > > index a89a806..690a88a 100644
> > > > --- a/drivers/acpi/acpi_fpdt.c
> > > > +++ b/drivers/acpi/acpi_fpdt.c
> > > > @@ -53,7 +53,7 @@ struct resume_performance_record {
> > > >  	u32 resume_count;
> > > >  	u64 resume_prev;
> > > >  	u64 resume_avg;
> > > > -} __attribute__((packed));
> > > > +} __packed;
> > > > 
> > > >  struct boot_performance_record {
> > > >  	struct fpdt_record_header header;
> > > > @@ -63,13 +63,13 @@ struct boot_performance_record {
> > > >  	u64 bootloader_launch;
> > > >  	u64 exitbootservice_start;
> > > >  	u64 exitbootservice_end;
> > > > -} __attribute__((packed));
> > > > +} __packed;
> > > > 
> > > >  struct suspend_performance_record {
> > > >  	struct fpdt_record_header header;
> > > >  	u64 suspend_start;
> > > >  	u64 suspend_end;
> > > > -} __attribute__((packed));
> > > > +} __packed;
> > > 
> > > My standard question about 'packed' is whether it is actually
> > > needed.
> > > It should only be used if the structures might be misaligned in
> > > memory.
> > > If the only problem is that a 64bit item needs to be 32bit
> > > aligned
> > > then a suitable type should be used for those specific fields.
> > > 
> > > Those all look very dubious - the standard header isn't packed
> > > so everything must eb assumed to be at least 32bit aligned.
> > > 
> > > There are also other sub-structures that contain 64bit values.
> > > These don't contain padding - but that requires 64bit alignement.
> > > 
> > > The only problematic structure is the last one - which would have
> > > a 32bit pad after the header.
> > > Is this even right given than there are explicit alignment pads
> > > in some of the other structures.
> > > 
> > > If 64bit alignment isn't guaranteed then a '64bit aligned to
> > > 32bit'
> > > type should be used for the u64 fields.
> > > 
> > 
> > Yes, some of them has been aligned already, then nothing changed
> > when 
> > add this "packed ". Maybe the purpose of the original author is
> > for 
> > extension, and can tell others that this struct need be packed.
> > 
> 
> The patch is upstreamed recently but it was made long time ago.
> I think the original problem is that one of the address, probably the
> suspend_performance record, is not 64bit aligned, thus we can not
> read
> the proper content of suspend_start and suspend_end, mapped from
> physical memory.
> 
> I will try to find a machine to reproduce the problem with all
> __attribute__((packed)) removed to double confirm this.
> 

So here is the problem, without __attribute__((packed))

[    0.858442] suspend_record: 0xffffaad500175020
/sys/firmware/acpi/fpdt/suspend/suspend_end_ns:addr:
0xffffaad500175030, 15998179292659843072
/sys/firmware/acpi/fpdt/suspend/suspend_start_ns:addr:
0xffffaad500175028, 0

suspend_record is mapped to 0xffffaad500175020, and it is combined with
one 32bit header and two 64bit fields (suspend_start and suspend_end),
this is how it is located in physical memory.
So the addresses of the two 64bit fields are actually not 64bit
aligned.

David,
Is this the "a 64bit item needs to be 32bit aligned" problem you
referred?
If yes, what is the proper fix? should I used two 32bits for each of
the field instead?

thanks,
rui

> thanks,
> rui
> > > 	David
> > > 
> > > -
> > > Registered Address Lakeside, Bramley Road, Mount Farm, Milton
> > > Keynes, MK1 1PT, UK
> > > Registration No: 1397386 (Wales)
> > > 
> > > 
> > > .
> > > 
> > 
> > 
> 
> 

RE: [PATCH v2 04/15] ACPI: table: replace __attribute__((packed)) by __packed

[David Laight]

From: Zhang Rui
> Sent: 30 March 2021 09:00
> To: Xiaofei Tan <tanxiaofei@huawei.com>; David Laight <David.Laight@ACULAB.COM>; rjw@rjwysocki.net;
> lenb@kernel.org; bhelgaas@google.com
> Cc: linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org;
> linuxarm@openeuler.org
> Subject: Re: [PATCH v2 04/15] ACPI: table: replace __attribute__((packed)) by __packed
> 
> On Tue, 2021-03-30 at 15:31 +0800, Zhang Rui wrote:
> > On Tue, 2021-03-30 at 10:23 +0800, Xiaofei Tan wrote:
> > > Hi David,
> > >
> > > On 2021/3/29 18:09, David Laight wrote:
> > > > From: Xiaofei Tan
> > > > > Sent: 27 March 2021 07:46
> > > > >
> > > > > Replace __attribute__((packed)) by __packed following the
> > > > > advice of checkpatch.pl.
> > > > >
> > > > > Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
> > > > > ---
> > > > >  drivers/acpi/acpi_fpdt.c | 6 +++---
> > > > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/drivers/acpi/acpi_fpdt.c
> > > > > b/drivers/acpi/acpi_fpdt.c
> > > > > index a89a806..690a88a 100644
> > > > > --- a/drivers/acpi/acpi_fpdt.c
> > > > > +++ b/drivers/acpi/acpi_fpdt.c
> > > > > @@ -53,7 +53,7 @@ struct resume_performance_record {
> > > > >  	u32 resume_count;
> > > > >  	u64 resume_prev;
> > > > >  	u64 resume_avg;
> > > > > -} __attribute__((packed));
> > > > > +} __packed;
> > > > >
> > > > >  struct boot_performance_record {
> > > > >  	struct fpdt_record_header header;
> > > > > @@ -63,13 +63,13 @@ struct boot_performance_record {
> > > > >  	u64 bootloader_launch;
> > > > >  	u64 exitbootservice_start;
> > > > >  	u64 exitbootservice_end;
> > > > > -} __attribute__((packed));
> > > > > +} __packed;
> > > > >
> > > > >  struct suspend_performance_record {
> > > > >  	struct fpdt_record_header header;
> > > > >  	u64 suspend_start;
> > > > >  	u64 suspend_end;
> > > > > -} __attribute__((packed));
> > > > > +} __packed;
> > > >
> > > > My standard question about 'packed' is whether it is actually
> > > > needed.
> > > > It should only be used if the structures might be misaligned in
> > > > memory.
> > > > If the only problem is that a 64bit item needs to be 32bit
> > > > aligned
> > > > then a suitable type should be used for those specific fields.
> > > >
> > > > Those all look very dubious - the standard header isn't packed
> > > > so everything must eb assumed to be at least 32bit aligned.
> > > >
> > > > There are also other sub-structures that contain 64bit values.
> > > > These don't contain padding - but that requires 64bit alignement.
> > > >
> > > > The only problematic structure is the last one - which would have
> > > > a 32bit pad after the header.
> > > > Is this even right given than there are explicit alignment pads
> > > > in some of the other structures.
> > > >
> > > > If 64bit alignment isn't guaranteed then a '64bit aligned to
> > > > 32bit'
> > > > type should be used for the u64 fields.
> > > >
> > >
> > > Yes, some of them has been aligned already, then nothing changed
> > > when
> > > add this "packed ". Maybe the purpose of the original author is
> > > for
> > > extension, and can tell others that this struct need be packed.
> > >
> >
> > The patch is upstreamed recently but it was made long time ago.
> > I think the original problem is that one of the address, probably the
> > suspend_performance record, is not 64bit aligned, thus we can not
> > read
> > the proper content of suspend_start and suspend_end, mapped from
> > physical memory.
> >
> > I will try to find a machine to reproduce the problem with all
> > __attribute__((packed)) removed to double confirm this.
> >
> 
> So here is the problem, without __attribute__((packed))
> 
> [    0.858442] suspend_record: 0xffffaad500175020
> /sys/firmware/acpi/fpdt/suspend/suspend_end_ns:addr:
> 0xffffaad500175030, 15998179292659843072
> /sys/firmware/acpi/fpdt/suspend/suspend_start_ns:addr:
> 0xffffaad500175028, 0
> 
> suspend_record is mapped to 0xffffaad500175020, and it is combined with
> one 32bit header and two 64bit fields (suspend_start and suspend_end),
> this is how it is located in physical memory.
> So the addresses of the two 64bit fields are actually not 64bit
> aligned.
> 
> David,
> Is this the "a 64bit item needs to be 32bit aligned" problem you
> referred?
> If yes, what is the proper fix? should I used two 32bits for each of
> the field instead?

Define something like:
typedef u64 __attribute__((aligned(4))) u64_align32;
and then use it for the 64bit structure members.

There doesn't seem to be a standard type name for it - although
it is used in several places.

I'm not entirely sure but is ACPI always LE?
(is it even x86 only??)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH v2 04/15] ACPI: table: replace __attribute__((packed)) by __packed

[Rafael J. Wysocki]

On Tue, Mar 30, 2021 at 10:15 AM David Laight <David.Laight@aculab.com> wrote:
>
> From: Zhang Rui
> > Sent: 30 March 2021 09:00
> > To: Xiaofei Tan <tanxiaofei@huawei.com>; David Laight <David.Laight@ACULAB.COM>; rjw@rjwysocki.net;
> > lenb@kernel.org; bhelgaas@google.com
> > Cc: linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org; linux-pci@vger.kernel.org;
> > linuxarm@openeuler.org
> > Subject: Re: [PATCH v2 04/15] ACPI: table: replace __attribute__((packed)) by __packed
> >
> > On Tue, 2021-03-30 at 15:31 +0800, Zhang Rui wrote:
> > > On Tue, 2021-03-30 at 10:23 +0800, Xiaofei Tan wrote:
> > > > Hi David,
> > > >
> > > > On 2021/3/29 18:09, David Laight wrote:
> > > > > From: Xiaofei Tan
> > > > > > Sent: 27 March 2021 07:46
> > > > > >
> > > > > > Replace __attribute__((packed)) by __packed following the
> > > > > > advice of checkpatch.pl.
> > > > > >
> > > > > > Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
> > > > > > ---
> > > > > >  drivers/acpi/acpi_fpdt.c | 6 +++---
> > > > > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/acpi/acpi_fpdt.c
> > > > > > b/drivers/acpi/acpi_fpdt.c
> > > > > > index a89a806..690a88a 100644
> > > > > > --- a/drivers/acpi/acpi_fpdt.c
> > > > > > +++ b/drivers/acpi/acpi_fpdt.c
> > > > > > @@ -53,7 +53,7 @@ struct resume_performance_record {
> > > > > >       u32 resume_count;
> > > > > >       u64 resume_prev;
> > > > > >       u64 resume_avg;
> > > > > > -} __attribute__((packed));
> > > > > > +} __packed;
> > > > > >
> > > > > >  struct boot_performance_record {
> > > > > >       struct fpdt_record_header header;
> > > > > > @@ -63,13 +63,13 @@ struct boot_performance_record {
> > > > > >       u64 bootloader_launch;
> > > > > >       u64 exitbootservice_start;
> > > > > >       u64 exitbootservice_end;
> > > > > > -} __attribute__((packed));
> > > > > > +} __packed;
> > > > > >
> > > > > >  struct suspend_performance_record {
> > > > > >       struct fpdt_record_header header;
> > > > > >       u64 suspend_start;
> > > > > >       u64 suspend_end;
> > > > > > -} __attribute__((packed));
> > > > > > +} __packed;
> > > > >
> > > > > My standard question about 'packed' is whether it is actually
> > > > > needed.
> > > > > It should only be used if the structures might be misaligned in
> > > > > memory.
> > > > > If the only problem is that a 64bit item needs to be 32bit
> > > > > aligned
> > > > > then a suitable type should be used for those specific fields.
> > > > >
> > > > > Those all look very dubious - the standard header isn't packed
> > > > > so everything must eb assumed to be at least 32bit aligned.
> > > > >
> > > > > There are also other sub-structures that contain 64bit values.
> > > > > These don't contain padding - but that requires 64bit alignement.
> > > > >
> > > > > The only problematic structure is the last one - which would have
> > > > > a 32bit pad after the header.
> > > > > Is this even right given than there are explicit alignment pads
> > > > > in some of the other structures.
> > > > >
> > > > > If 64bit alignment isn't guaranteed then a '64bit aligned to
> > > > > 32bit'
> > > > > type should be used for the u64 fields.
> > > > >
> > > >
> > > > Yes, some of them has been aligned already, then nothing changed
> > > > when
> > > > add this "packed ". Maybe the purpose of the original author is
> > > > for
> > > > extension, and can tell others that this struct need be packed.
> > > >
> > >
> > > The patch is upstreamed recently but it was made long time ago.
> > > I think the original problem is that one of the address, probably the
> > > suspend_performance record, is not 64bit aligned, thus we can not
> > > read
> > > the proper content of suspend_start and suspend_end, mapped from
> > > physical memory.
> > >
> > > I will try to find a machine to reproduce the problem with all
> > > __attribute__((packed)) removed to double confirm this.
> > >
> >
> > So here is the problem, without __attribute__((packed))
> >
> > [    0.858442] suspend_record: 0xffffaad500175020
> > /sys/firmware/acpi/fpdt/suspend/suspend_end_ns:addr:
> > 0xffffaad500175030, 15998179292659843072
> > /sys/firmware/acpi/fpdt/suspend/suspend_start_ns:addr:
> > 0xffffaad500175028, 0
> >
> > suspend_record is mapped to 0xffffaad500175020, and it is combined with
> > one 32bit header and two 64bit fields (suspend_start and suspend_end),
> > this is how it is located in physical memory.
> > So the addresses of the two 64bit fields are actually not 64bit
> > aligned.
> >
> > David,
> > Is this the "a 64bit item needs to be 32bit aligned" problem you
> > referred?
> > If yes, what is the proper fix? should I used two 32bits for each of
> > the field instead?
>
> Define something like:
> typedef u64 __attribute__((aligned(4))) u64_align32;
> and then use it for the 64bit structure members.
>
> There doesn't seem to be a standard type name for it - although
> it is used in several places.
>
> I'm not entirely sure but is ACPI always LE?

Yes.

> (is it even x86 only??)

No.

Re: [PATCH v2 04/15] ACPI: table: replace __attribute__((packed)) by __packed

[Zhang Rui]

On Tue, 2021-03-30 at 08:14 +0000, David Laight wrote:
> From: Zhang Rui
> > Sent: 30 March 2021 09:00
> > To: Xiaofei Tan <tanxiaofei@huawei.com>; David Laight <
> > David.Laight@ACULAB.COM>; rjw@rjwysocki.net;
> > lenb@kernel.org; bhelgaas@google.com
> > Cc: linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org; 
> > linux-pci@vger.kernel.org;
> > linuxarm@openeuler.org
> > Subject: Re: [PATCH v2 04/15] ACPI: table: replace
> > __attribute__((packed)) by __packed
> > 
> > On Tue, 2021-03-30 at 15:31 +0800, Zhang Rui wrote:
> > > On Tue, 2021-03-30 at 10:23 +0800, Xiaofei Tan wrote:
> > > > Hi David,
> > > > 
> > > > On 2021/3/29 18:09, David Laight wrote:
> > > > > From: Xiaofei Tan
> > > > > > Sent: 27 March 2021 07:46
> > > > > > 
> > > > > > Replace __attribute__((packed)) by __packed following the
> > > > > > advice of checkpatch.pl.
> > > > > > 
> > > > > > Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
> > > > > > ---
> > > > > >  drivers/acpi/acpi_fpdt.c | 6 +++---
> > > > > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/acpi/acpi_fpdt.c
> > > > > > b/drivers/acpi/acpi_fpdt.c
> > > > > > index a89a806..690a88a 100644
> > > > > > --- a/drivers/acpi/acpi_fpdt.c
> > > > > > +++ b/drivers/acpi/acpi_fpdt.c
> > > > > > @@ -53,7 +53,7 @@ struct resume_performance_record {
> > > > > >  	u32 resume_count;
> > > > > >  	u64 resume_prev;
> > > > > >  	u64 resume_avg;
> > > > > > -} __attribute__((packed));
> > > > > > +} __packed;
> > > > > > 
> > > > > >  struct boot_performance_record {
> > > > > >  	struct fpdt_record_header header;
> > > > > > @@ -63,13 +63,13 @@ struct boot_performance_record {
> > > > > >  	u64 bootloader_launch;
> > > > > >  	u64 exitbootservice_start;
> > > > > >  	u64 exitbootservice_end;
> > > > > > -} __attribute__((packed));
> > > > > > +} __packed;
> > > > > > 
> > > > > >  struct suspend_performance_record {
> > > > > >  	struct fpdt_record_header header;
> > > > > >  	u64 suspend_start;
> > > > > >  	u64 suspend_end;
> > > > > > -} __attribute__((packed));
> > > > > > +} __packed;
> > > > > 
> > > > > My standard question about 'packed' is whether it is actually
> > > > > needed.
> > > > > It should only be used if the structures might be misaligned
> > > > > in
> > > > > memory.
> > > > > If the only problem is that a 64bit item needs to be 32bit
> > > > > aligned
> > > > > then a suitable type should be used for those specific
> > > > > fields.
> > > > > 
> > > > > Those all look very dubious - the standard header isn't
> > > > > packed
> > > > > so everything must eb assumed to be at least 32bit aligned.
> > > > > 
> > > > > There are also other sub-structures that contain 64bit
> > > > > values.
> > > > > These don't contain padding - but that requires 64bit
> > > > > alignement.
> > > > > 
> > > > > The only problematic structure is the last one - which would
> > > > > have
> > > > > a 32bit pad after the header.
> > > > > Is this even right given than there are explicit alignment
> > > > > pads
> > > > > in some of the other structures.
> > > > > 
> > > > > If 64bit alignment isn't guaranteed then a '64bit aligned to
> > > > > 32bit'
> > > > > type should be used for the u64 fields.
> > > > > 
> > > > 
> > > > Yes, some of them has been aligned already, then nothing
> > > > changed
> > > > when
> > > > add this "packed ". Maybe the purpose of the original author is
> > > > for
> > > > extension, and can tell others that this struct need be packed.
> > > > 
> > > 
> > > The patch is upstreamed recently but it was made long time ago.
> > > I think the original problem is that one of the address, probably
> > > the
> > > suspend_performance record, is not 64bit aligned, thus we can not
> > > read
> > > the proper content of suspend_start and suspend_end, mapped from
> > > physical memory.
> > > 
> > > I will try to find a machine to reproduce the problem with all
> > > __attribute__((packed)) removed to double confirm this.
> > > 
> > 
> > So here is the problem, without __attribute__((packed))
> > 
> > [    0.858442] suspend_record: 0xffffaad500175020
> > /sys/firmware/acpi/fpdt/suspend/suspend_end_ns:addr:
> > 0xffffaad500175030, 15998179292659843072
> > /sys/firmware/acpi/fpdt/suspend/suspend_start_ns:addr:
> > 0xffffaad500175028, 0
> > 
> > suspend_record is mapped to 0xffffaad500175020, and it is combined
> > with
> > one 32bit header and two 64bit fields (suspend_start and
> > suspend_end),
> > this is how it is located in physical memory.
> > So the addresses of the two 64bit fields are actually not 64bit
> > aligned.
> > 
> > David,
> > Is this the "a 64bit item needs to be 32bit aligned" problem you
> > referred?
> > If yes, what is the proper fix? should I used two 32bits for each
> > of
> > the field instead?
> 
> Define something like:
> typedef u64 __attribute__((aligned(4))) u64_align32;
> and then use it for the 64bit structure members.
> 
Hi, David,

Please kindly help check if the following patch is the right fix or
not. I've verified it to work on my test box.

The reason I use this typedef for all the u64 items because there is no
guarantee that the suspend_performance record is in the end of the
memory, thus it may pollute the others.

From e18c942855e2f51e814d057fff4dd951cd0d0907 Mon Sep 17 00:00:00 2001
From: Zhang Rui <rui.zhang@intel.com>
Date: Wed, 31 Mar 2021 20:34:13 +0800
Subject: [PATCH] ACPI: tables: FPDT: Fix 64bit alignment issue

Some of the 64bit items in FPDT table may be 32bit aligned.
Using __attribute__((packed)) is not needed in this case, fixing it by
allowing 32bit alignment for these 64bit items.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/acpi/acpi_fpdt.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/acpi/acpi_fpdt.c b/drivers/acpi/acpi_fpdt.c
index a89a806a7a2a..94e107b9a114 100644
--- a/drivers/acpi/acpi_fpdt.c
+++ b/drivers/acpi/acpi_fpdt.c
@@ -23,12 +23,14 @@ enum fpdt_subtable_type {
 	SUBTABLE_S3PT,
 };
 
+typedef u64 __attribute__((aligned(4))) u64_align32;
+
 struct fpdt_subtable_entry {
 	u16 type;		/* refer to enum fpdt_subtable_type */
 	u8 length;
 	u8 revision;
 	u32 reserved;
-	u64 address;		/* physical address of the S3PT/FBPT table */
+	u64_align32 address;		/* physical address of the S3PT/FBPT table */
 };
 
 struct fpdt_subtable_header {
@@ -51,25 +53,25 @@ struct fpdt_record_header {
 struct resume_performance_record {
 	struct fpdt_record_header header;
 	u32 resume_count;
-	u64 resume_prev;
-	u64 resume_avg;
-} __attribute__((packed));
+	u64_align32 resume_prev;
+	u64_align32 resume_avg;
+};
 
 struct boot_performance_record {
 	struct fpdt_record_header header;
 	u32 reserved;
-	u64 firmware_start;
-	u64 bootloader_load;
-	u64 bootloader_launch;
-	u64 exitbootservice_start;
-	u64 exitbootservice_end;
-} __attribute__((packed));
+	u64_align32 firmware_start;
+	u64_align32 bootloader_load;
+	u64_align32 bootloader_launch;
+	u64_align32 exitbootservice_start;
+	u64_align32 exitbootservice_end;
+};
 
 struct suspend_performance_record {
 	struct fpdt_record_header header;
-	u64 suspend_start;
-	u64 suspend_end;
-} __attribute__((packed));
+	u64_align32 suspend_start;
+	u64_align32 suspend_end;
+};
 
 
 static struct resume_performance_record *record_resume;
-- 
2.17.1

RE: [PATCH v2 04/15] ACPI: table: replace __attribute__((packed)) by __packed

[David Laight]

From: Zhang Rui
> Sent: 31 March 2021 16:55
> On Tue, 2021-03-30 at 08:14 +0000, David Laight wrote:
> > From: Zhang Rui
> > > Sent: 30 March 2021 09:00
> > > > On Tue, 2021-03-30 at 10:23 +0800, Xiaofei Tan wrote:
> > > > > Hi David,
> > > > >
> > > > > On 2021/3/29 18:09, David Laight wrote:
> > > > > > From: Xiaofei Tan
> > > > > > > Sent: 27 March 2021 07:46
> > > > > > >
> > > > > > > Replace __attribute__((packed)) by __packed following the
> > > > > > > advice of checkpatch.pl.
> > > > > > >
> > > > > > > Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
> > > > > > > ---
> > > > > > >  drivers/acpi/acpi_fpdt.c | 6 +++---
> > > > > > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/acpi/acpi_fpdt.c
> > > > > > > b/drivers/acpi/acpi_fpdt.c
> > > > > > > index a89a806..690a88a 100644
> > > > > > > --- a/drivers/acpi/acpi_fpdt.c
> > > > > > > +++ b/drivers/acpi/acpi_fpdt.c
> > > > > > > @@ -53,7 +53,7 @@ struct resume_performance_record {
> > > > > > >  	u32 resume_count;
> > > > > > >  	u64 resume_prev;
> > > > > > >  	u64 resume_avg;
> > > > > > > -} __attribute__((packed));
> > > > > > > +} __packed;
> > > > > > >
> > > > > > >  struct boot_performance_record {
> > > > > > >  	struct fpdt_record_header header;
> > > > > > > @@ -63,13 +63,13 @@ struct boot_performance_record {
> > > > > > >  	u64 bootloader_launch;
> > > > > > >  	u64 exitbootservice_start;
> > > > > > >  	u64 exitbootservice_end;
> > > > > > > -} __attribute__((packed));
> > > > > > > +} __packed;
> > > > > > >
> > > > > > >  struct suspend_performance_record {
> > > > > > >  	struct fpdt_record_header header;
> > > > > > >  	u64 suspend_start;
> > > > > > >  	u64 suspend_end;
> > > > > > > -} __attribute__((packed));
> > > > > > > +} __packed;
> > > > > >
> > > > > > My standard question about 'packed' is whether it is actually
> > > > > > needed.
> > > > > > It should only be used if the structures might be misaligned
> > > > > > in
> > > > > > memory.
> > > > > > If the only problem is that a 64bit item needs to be 32bit
> > > > > > aligned
> > > > > > then a suitable type should be used for those specific
> > > > > > fields.
> > > > > >
> > > > > > Those all look very dubious - the standard header isn't
> > > > > > packed
> > > > > > so everything must eb assumed to be at least 32bit aligned.
> > > > > >
> > > > > > There are also other sub-structures that contain 64bit
> > > > > > values.
> > > > > > These don't contain padding - but that requires 64bit
> > > > > > alignement.
> > > > > >
> > > > > > The only problematic structure is the last one - which would
> > > > > > have
> > > > > > a 32bit pad after the header.
> > > > > > Is this even right given than there are explicit alignment
> > > > > > pads
> > > > > > in some of the other structures.
> > > > > >
> > > > > > If 64bit alignment isn't guaranteed then a '64bit aligned to
> > > > > > 32bit'
> > > > > > type should be used for the u64 fields.
> > > > > >
> > > > >
> > > > > Yes, some of them has been aligned already, then nothing
> > > > > changed
> > > > > when
> > > > > add this "packed ". Maybe the purpose of the original author is
> > > > > for
> > > > > extension, and can tell others that this struct need be packed.
> > > > >
> > > >
> > > > The patch is upstreamed recently but it was made long time ago.
> > > > I think the original problem is that one of the address, probably
> > > > the
> > > > suspend_performance record, is not 64bit aligned, thus we can not
> > > > read
> > > > the proper content of suspend_start and suspend_end, mapped from
> > > > physical memory.
> > > >
> > > > I will try to find a machine to reproduce the problem with all
> > > > __attribute__((packed)) removed to double confirm this.
> > > >
> > >
> > > So here is the problem, without __attribute__((packed))
> > >
> > > [    0.858442] suspend_record: 0xffffaad500175020
> > > /sys/firmware/acpi/fpdt/suspend/suspend_end_ns:addr:
> > > 0xffffaad500175030, 15998179292659843072
> > > /sys/firmware/acpi/fpdt/suspend/suspend_start_ns:addr:
> > > 0xffffaad500175028, 0
> > >
> > > suspend_record is mapped to 0xffffaad500175020, and it is combined
> > > with
> > > one 32bit header and two 64bit fields (suspend_start and
> > > suspend_end),
> > > this is how it is located in physical memory.
> > > So the addresses of the two 64bit fields are actually not 64bit
> > > aligned.
> > >
> > > David,
> > > Is this the "a 64bit item needs to be 32bit aligned" problem you
> > > referred?
> > > If yes, what is the proper fix? should I used two 32bits for each
> > > of
> > > the field instead?
> >
> > Define something like:
> > typedef u64 __attribute__((aligned(4))) u64_align32;
> > and then use it for the 64bit structure members.
> >
> Hi, David,
> 
> Please kindly help check if the following patch is the right fix or
> not. I've verified it to work on my test box.
> 
> The reason I use this typedef for all the u64 items because there is no
> guarantee that the suspend_performance record is in the end of the
> memory, thus it may pollute the others.

Looks about right.

	David

> 
> From e18c942855e2f51e814d057fff4dd951cd0d0907 Mon Sep 17 00:00:00 2001
> From: Zhang Rui <rui.zhang@intel.com>
> Date: Wed, 31 Mar 2021 20:34:13 +0800
> Subject: [PATCH] ACPI: tables: FPDT: Fix 64bit alignment issue
> 
> Some of the 64bit items in FPDT table may be 32bit aligned.
> Using __attribute__((packed)) is not needed in this case, fixing it by
> allowing 32bit alignment for these 64bit items.
> 
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  drivers/acpi/acpi_fpdt.c | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/acpi/acpi_fpdt.c b/drivers/acpi/acpi_fpdt.c
> index a89a806a7a2a..94e107b9a114 100644
> --- a/drivers/acpi/acpi_fpdt.c
> +++ b/drivers/acpi/acpi_fpdt.c
> @@ -23,12 +23,14 @@ enum fpdt_subtable_type {
>  	SUBTABLE_S3PT,
>  };
> 
> +typedef u64 __attribute__((aligned(4))) u64_align32;
> +
>  struct fpdt_subtable_entry {
>  	u16 type;		/* refer to enum fpdt_subtable_type */
>  	u8 length;
>  	u8 revision;
>  	u32 reserved;
> -	u64 address;		/* physical address of the S3PT/FBPT table */
> +	u64_align32 address;		/* physical address of the S3PT/FBPT table */
>  };
> 
>  struct fpdt_subtable_header {
> @@ -51,25 +53,25 @@ struct fpdt_record_header {
>  struct resume_performance_record {
>  	struct fpdt_record_header header;
>  	u32 resume_count;
> -	u64 resume_prev;
> -	u64 resume_avg;
> -} __attribute__((packed));
> +	u64_align32 resume_prev;
> +	u64_align32 resume_avg;
> +};
> 
>  struct boot_performance_record {
>  	struct fpdt_record_header header;
>  	u32 reserved;
> -	u64 firmware_start;
> -	u64 bootloader_load;
> -	u64 bootloader_launch;
> -	u64 exitbootservice_start;
> -	u64 exitbootservice_end;
> -} __attribute__((packed));
> +	u64_align32 firmware_start;
> +	u64_align32 bootloader_load;
> +	u64_align32 bootloader_launch;
> +	u64_align32 exitbootservice_start;
> +	u64_align32 exitbootservice_end;
> +};
> 
>  struct suspend_performance_record {
>  	struct fpdt_record_header header;
> -	u64 suspend_start;
> -	u64 suspend_end;
> -} __attribute__((packed));
> +	u64_align32 suspend_start;
> +	u64_align32 suspend_end;
> +};
> 
> 
>  static struct resume_performance_record *record_resume;
> --
> 2.17.1
> 

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH v2 04/15] ACPI: table: replace __attribute__((packed)) by __packed

[Bjorn Helgaas]

On Wed, Mar 31, 2021 at 11:55:08PM +0800, Zhang Rui wrote:
> ...

> From e18c942855e2f51e814d057fff4dd951cd0d0907 Mon Sep 17 00:00:00 2001
> From: Zhang Rui <rui.zhang@intel.com>
> Date: Wed, 31 Mar 2021 20:34:13 +0800
> Subject: [PATCH] ACPI: tables: FPDT: Fix 64bit alignment issue
> 
> Some of the 64bit items in FPDT table may be 32bit aligned.
> Using __attribute__((packed)) is not needed in this case, fixing it by
> allowing 32bit alignment for these 64bit items.

1) Can you please add a spec reference for this?  I think it's ACPI
   v6.3, sec 5.2.23.5, or something close to that.

2) The exact layout in memory is prescribed by the spec.  I think
   that's basically what "packed" accomplishes.  I don't understand
   why using "aligned" would be preferable.  Using "aligned" means
   things can be at different offsets depending on the starting
   address of the structure.  We always want the identical layout, no
   matter what the starting address is.

> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  drivers/acpi/acpi_fpdt.c | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/acpi/acpi_fpdt.c b/drivers/acpi/acpi_fpdt.c
> index a89a806a7a2a..94e107b9a114 100644
> --- a/drivers/acpi/acpi_fpdt.c
> +++ b/drivers/acpi/acpi_fpdt.c
> @@ -23,12 +23,14 @@ enum fpdt_subtable_type {
>  	SUBTABLE_S3PT,
>  };
>  
> +typedef u64 __attribute__((aligned(4))) u64_align32;
> +
>  struct fpdt_subtable_entry {
>  	u16 type;		/* refer to enum fpdt_subtable_type */
>  	u8 length;
>  	u8 revision;
>  	u32 reserved;
> -	u64 address;		/* physical address of the S3PT/FBPT table */
> +	u64_align32 address;		/* physical address of the S3PT/FBPT table */
>  };
>  
>  struct fpdt_subtable_header {
> @@ -51,25 +53,25 @@ struct fpdt_record_header {
>  struct resume_performance_record {
>  	struct fpdt_record_header header;
>  	u32 resume_count;
> -	u64 resume_prev;
> -	u64 resume_avg;
> -} __attribute__((packed));
> +	u64_align32 resume_prev;
> +	u64_align32 resume_avg;
> +};
>  
>  struct boot_performance_record {
>  	struct fpdt_record_header header;
>  	u32 reserved;
> -	u64 firmware_start;
> -	u64 bootloader_load;
> -	u64 bootloader_launch;
> -	u64 exitbootservice_start;
> -	u64 exitbootservice_end;
> -} __attribute__((packed));
> +	u64_align32 firmware_start;
> +	u64_align32 bootloader_load;
> +	u64_align32 bootloader_launch;
> +	u64_align32 exitbootservice_start;
> +	u64_align32 exitbootservice_end;
> +};
>  
>  struct suspend_performance_record {
>  	struct fpdt_record_header header;
> -	u64 suspend_start;
> -	u64 suspend_end;
> -} __attribute__((packed));
> +	u64_align32 suspend_start;
> +	u64_align32 suspend_end;
> +};
>  
>  
>  static struct resume_performance_record *record_resume;
> -- 
> 2.17.1
> 
> 

RE: [PATCH v2 04/15] ACPI: table: replace __attribute__((packed)) by __packed

[David Laight]

From: Bjorn Helgaas
> Sent: 31 March 2021 18:22
> 
> On Wed, Mar 31, 2021 at 11:55:08PM +0800, Zhang Rui wrote:
> > ...
> 
> > From e18c942855e2f51e814d057fff4dd951cd0d0907 Mon Sep 17 00:00:00 2001
> > From: Zhang Rui <rui.zhang@intel.com>
> > Date: Wed, 31 Mar 2021 20:34:13 +0800
> > Subject: [PATCH] ACPI: tables: FPDT: Fix 64bit alignment issue
> >
> > Some of the 64bit items in FPDT table may be 32bit aligned.
> > Using __attribute__((packed)) is not needed in this case, fixing it by
> > allowing 32bit alignment for these 64bit items.
> 
> 1) Can you please add a spec reference for this?  I think it's ACPI
>    v6.3, sec 5.2.23.5, or something close to that.
> 
> 2) The exact layout in memory is prescribed by the spec.  I think
>    that's basically what "packed" accomplishes.  I don't understand
>    why using "aligned" would be preferable.  Using "aligned" means
>    things can be at different offsets depending on the starting
>    address of the structure.  We always want the identical layout, no
>    matter what the starting address is.

Both 'packed' and 'aligned(4)' remove any structure alignment
padding before 64bit items that aren't on an 8 byte boundary.
(Because everything else in the structures is naturally aligned.)

The difference is significant on cpu that don't support misaligned
addresses.
Assuming that the structure is always on a 4n byte boundary
(which the ACPI spec probably requires) accesses to the 32-bit
fields are always ok.
It is only 64-bit fields that must be accessed as two 32-bit
memory cycles, not all the fields using multiple single byte
cycles.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH v2 04/15] ACPI: table: replace __attribute__((packed)) by __packed

[Rafael J. Wysocki]

On Thu, Apr 1, 2021 at 11:00 AM David Laight <David.Laight@aculab.com> wrote:
>
> From: Bjorn Helgaas
> > Sent: 31 March 2021 18:22
> >
> > On Wed, Mar 31, 2021 at 11:55:08PM +0800, Zhang Rui wrote:
> > > ...
> >
> > > From e18c942855e2f51e814d057fff4dd951cd0d0907 Mon Sep 17 00:00:00 2001
> > > From: Zhang Rui <rui.zhang@intel.com>
> > > Date: Wed, 31 Mar 2021 20:34:13 +0800
> > > Subject: [PATCH] ACPI: tables: FPDT: Fix 64bit alignment issue
> > >
> > > Some of the 64bit items in FPDT table may be 32bit aligned.
> > > Using __attribute__((packed)) is not needed in this case, fixing it by
> > > allowing 32bit alignment for these 64bit items.
> >
> > 1) Can you please add a spec reference for this?  I think it's ACPI
> >    v6.3, sec 5.2.23.5, or something close to that.
> >
> > 2) The exact layout in memory is prescribed by the spec.  I think
> >    that's basically what "packed" accomplishes.  I don't understand
> >    why using "aligned" would be preferable.  Using "aligned" means
> >    things can be at different offsets depending on the starting
> >    address of the structure.  We always want the identical layout, no
> >    matter what the starting address is.
>
> Both 'packed' and 'aligned(4)' remove any structure alignment
> padding before 64bit items that aren't on an 8 byte boundary.
> (Because everything else in the structures is naturally aligned.)
>
> The difference is significant on cpu that don't support misaligned
> addresses.
> Assuming that the structure is always on a 4n byte boundary
> (which the ACPI spec probably requires) accesses to the 32-bit
> fields are always ok.
> It is only 64-bit fields that must be accessed as two 32-bit
> memory cycles, not all the fields using multiple single byte
> cycles.

So what exactly is wrong with using "packed"?  It is way easier to
understand for a casual reader of the code.

RE: [PATCH v2 04/15] ACPI: table: replace __attribute__((packed)) by __packed

[David Laight]

From: Rafael J. Wysocki
> Sent: 01 April 2021 14:50
...
> So what exactly is wrong with using "packed"?  It is way easier to
> understand for a casual reader of the code.

Because it is usually wrong!

If I have:
	struct foo {
		u64 val;
	} __packed;

And then have:
u64 bar(struct foo *foo)
{
	return foo->val;
}

The on some cpu the compiler has to generate the equivalent of:
	u8 *x = (void *)&foo->val;
	return x[0] | x[1] << 8 | x[2] << 16 | x[3] << 24 | x[4] << 32 | x[5] << 40 | x[6] << 48 | x[7] << 56;

If you can guarantee that the structure is 32bit aligned
then it can generate the simpler:
	u32 *x = (void *)&foo->val;
	return x[0] | x[1] << 32;

(Yes I've missed out the 64-bit casts)

This is why you should almost never use __packed.

There are historic structures with 64 bit items on 4 byte boundaries
(and 32 bit values on 2 byte boundaries).
Typically most of the fields are shorter so can be read directly
(although they might need a byte-swapping load).

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH v2 04/15] ACPI: table: replace __attribute__((packed)) by __packed

[Rafael J. Wysocki]

On Thu, Apr 1, 2021 at 4:23 PM David Laight <David.Laight@aculab.com> wrote:
>
> From: Rafael J. Wysocki
> > Sent: 01 April 2021 14:50
> ...
> > So what exactly is wrong with using "packed"?  It is way easier to
> > understand for a casual reader of the code.
>
> Because it is usually wrong!
>
> If I have:
>         struct foo {
>                 u64 val;
>         } __packed;
>
> And then have:
> u64 bar(struct foo *foo)
> {
>         return foo->val;
> }
>
> The on some cpu the compiler has to generate the equivalent of:
>         u8 *x = (void *)&foo->val;
>         return x[0] | x[1] << 8 | x[2] << 16 | x[3] << 24 | x[4] << 32 | x[5] << 40 | x[6] << 48 | x[7] << 56;
>
> If you can guarantee that the structure is 32bit aligned
> then it can generate the simpler:
>         u32 *x = (void *)&foo->val;
>         return x[0] | x[1] << 32;
>
> (Yes I've missed out the 64-bit casts)
>
> This is why you should almost never use __packed.
>
> There are historic structures with 64 bit items on 4 byte boundaries
> (and 32 bit values on 2 byte boundaries).
> Typically most of the fields are shorter so can be read directly
> (although they might need a byte-swapping load).

The possible overhead impact is clear to me, but I really don't like
the "local" typedef idea.

It at least would need to be accompanied by a comment explaining why
it is there and why using it is better than using __packed and why
this needs to be defined locally and not in some generic header file.

Also, the FPDT code is just one function that parses the entire table
and there is no object passing between functions in it etc, so is
__packed still problematic in there?

[PATCH v2 05/15] ACPI: ipmi: remove useless return statement for void function

[Xiaofei Tan]

Remove useless return statement for void function, reported by
checkpatch.pl.

WARNING: void function return statements are not generally useful
FILE: drivers/acpi/acpi_ipmi.c:482:
+       return;
+}

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
---
 drivers/acpi/acpi_ipmi.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c
index 9d6c0fc..bbd00d9 100644
--- a/drivers/acpi/acpi_ipmi.c
+++ b/drivers/acpi/acpi_ipmi.c
@@ -478,7 +478,6 @@ static void ipmi_register_bmc(int iface, struct device *dev)
 	ipmi_dev_release(ipmi_device);
 err_ref:
 	put_device(smi_data.dev);
-	return;
 }
 
 static void ipmi_bmc_gone(int iface)
-- 
2.8.1

[PATCH v2 06/15] ACPI: LPSS: fix some coding style issues

[Xiaofei Tan]

Fix some coding style issues reported by checkpatch.pl, including
following types:

WARNING: simple_strtol is obsolete, use kstrtol instead
WARNING: Missing a blank line after declarations

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
---
 drivers/acpi/acpi_lpss.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
index be73974..2df231e 100644
--- a/drivers/acpi/acpi_lpss.c
+++ b/drivers/acpi/acpi_lpss.c
@@ -187,7 +187,7 @@ static void byt_i2c_setup(struct lpss_private_data *pdata)
 
 	/* Expected to always be true, but better safe then sorry */
 	if (uid_str)
-		uid = simple_strtol(uid_str, NULL, 10);
+		uid = kstrtol(uid_str, NULL, 10);
 
 	/* Detect I2C bus shared with PUNIT and ignore its d3 status */
 	status = acpi_evaluate_integer(handle, "_SEM", NULL, &shared_host);
@@ -377,6 +377,7 @@ static const struct acpi_device_id acpi_lpss_device_ids[] = {
 static int is_memory(struct acpi_resource *res, void *not_used)
 {
 	struct resource r;
+
 	return !acpi_dev_resource_memory(res, &r);
 }
 
@@ -1200,6 +1201,7 @@ static int acpi_lpss_poweroff_noirq(struct device *dev)
 	if (pdata->dev_desc->resume_from_noirq) {
 		/* This is analogous to the acpi_lpss_suspend_noirq() case. */
 		int ret = acpi_lpss_do_poweroff_late(dev);
+
 		if (ret)
 			return ret;
 	}
-- 
2.8.1

Re: [PATCH v2 06/15] ACPI: LPSS: fix some coding style issues

[Xiaofei Tan]

Hi Andy,

On 2021/3/27 16:17, Andy Shevchenko wrote:
>
>
> On Saturday, March 27, 2021, Xiaofei Tan <tanxiaofei@huawei.com
> <mailto:tanxiaofei@huawei.com>> wrote:
>
>     Fix some coding style issues reported by checkpatch.pl
>     <http://checkpatch.pl>, including
>     following types:
>
>     WARNING: simple_strtol is obsolete, use kstrtol instead
>     WARNING: Missing a blank line after declarations
>
>
>
> First of all, two changes ==> two patches.
>

I thought it would be better to include more simple cleanup in one patch.

>
>
>     Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com
>     <mailto:tanxiaofei@huawei.com>>
>     ---
>      drivers/acpi/acpi_lpss.c | 4 +++-
>      1 file changed, 3 insertions(+), 1 deletion(-)
>
>     diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
>     index be73974..2df231e 100644
>     --- a/drivers/acpi/acpi_lpss.c
>     +++ b/drivers/acpi/acpi_lpss.c
>     @@ -187,7 +187,7 @@ static void byt_i2c_setup(struct
>     lpss_private_data *pdata)
>
>             /* Expected to always be true, but better safe then sorry */
>             if (uid_str)
>     -               uid = simple_strtol(uid_str, NULL, 10);
>     +               uid = kstrtol(uid_str, NULL, 10);
>
>
> How did you test this? Is there any guarantee that input is
> null-terminated number?
>
> Where is the check of returned value from `kstrtol()`?
>
> Yes, you haven’t tested that at all. Don’t submit patches you are not
> able to test and haven’t tested.
>

It's my fault. Sorry for that, i will fix it.

> NAK.
>
>
>             /* Detect I2C bus shared with PUNIT and ignore its d3 status */
>             status = acpi_evaluate_integer(handle, "_SEM", NULL,
>     &shared_host);
>     @@ -377,6 +377,7 @@ static const struct acpi_device_id
>     acpi_lpss_device_ids[] = {
>      static int is_memory(struct acpi_resource *res, void *not_used)
>      {
>             struct resource r;
>     +
>             return !acpi_dev_resource_memory(res, &r);
>      }
>
>     @@ -1200,6 +1201,7 @@ static int acpi_lpss_poweroff_noirq(struct
>     device *dev)
>             if (pdata->dev_desc->resume_from_noirq) {
>                     /* This is analogous to the
>     acpi_lpss_suspend_noirq() case. */
>                     int ret = acpi_lpss_do_poweroff_late(dev);
>     +
>                     if (ret)
>                             return ret;
>             }
>     --
>     2.8.1
>
>
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Re: [PATCH v2 06/15] ACPI: LPSS: fix some coding style issues

[Xiaofei Tan]

Hi Andy,

On 2021/3/27 16:19, Andy Shevchenko wrote:
>
>
> On Saturday, March 27, 2021, Xiaofei Tan <tanxiaofei@huawei.com
> <mailto:tanxiaofei@huawei.com>> wrote:
>
>     Fix some coding style issues reported by checkpatch.pl
>     <http://checkpatch.pl>, including
>     following types:
>
>     WARNING: simple_strtol is obsolete, use kstrtol instead
>
>
> And one more thing, the above message is bogus. Read what the comments
> in the code says about use cases for simple_*() vs. kstrto*() ones.
>

OK. I would remove this modification from the patch.

> Joe?
>
>
>     WARNING: Missing a blank line after declarations
>
>     Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com
>     <mailto:tanxiaofei@huawei.com>>
>     ---
>      drivers/acpi/acpi_lpss.c | 4 +++-
>      1 file changed, 3 insertions(+), 1 deletion(-)
>
>     diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
>     index be73974..2df231e 100644
>     --- a/drivers/acpi/acpi_lpss.c
>     +++ b/drivers/acpi/acpi_lpss.c
>     @@ -187,7 +187,7 @@ static void byt_i2c_setup(struct
>     lpss_private_data *pdata)
>
>             /* Expected to always be true, but better safe then sorry */
>             if (uid_str)
>     -               uid = simple_strtol(uid_str, NULL, 10);
>     +               uid = kstrtol(uid_str, NULL, 10);
>
>             /* Detect I2C bus shared with PUNIT and ignore its d3 status */
>             status = acpi_evaluate_integer(handle, "_SEM", NULL,
>     &shared_host);
>     @@ -377,6 +377,7 @@ static const struct acpi_device_id
>     acpi_lpss_device_ids[] = {
>      static int is_memory(struct acpi_resource *res, void *not_used)
>      {
>             struct resource r;
>     +
>             return !acpi_dev_resource_memory(res, &r);
>      }
>
>     @@ -1200,6 +1201,7 @@ static int acpi_lpss_poweroff_noirq(struct
>     device *dev)
>             if (pdata->dev_desc->resume_from_noirq) {
>                     /* This is analogous to the
>     acpi_lpss_suspend_noirq() case. */
>                     int ret = acpi_lpss_do_poweroff_late(dev);
>     +
>                     if (ret)
>                             return ret;
>             }
>     --
>     2.8.1
>
>
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Re: [PATCH v2 06/15] ACPI: LPSS: fix some coding style issues

[Joe Perches]

On Sat, 2021-03-27 at 10:19 +0200, Andy Shevchenko wrote:
> On Saturday, March 27, 2021, Xiaofei Tan <tanxiaofei@huawei.com> wrote:
> 
> > Fix some coding style issues reported by checkpatch.pl, including
> > following types:
> > 
> > WARNING: simple_strtol is obsolete, use kstrtol instead
> 
> 
> And one more thing, the above message is bogus. Read what the comments in
> the code says about use cases for simple_*() vs. kstrto*() ones.
> 
> Joe?

This check and message is nearly 10 years old and was appropriate for
when it was implemented.

kernel.h currently has:
 * Use these functions if and only if you cannot use kstrto<foo>, because

So the message could be changed to something like:

WARNING: simple_strtol should be used only when kstrtol can not be used.

Re: [PATCH v2 06/15] ACPI: LPSS: fix some coding style issues

[Andy Shevchenko]

On Sat, Mar 27, 2021 at 3:39 PM Joe Perches <joe@perches.com> wrote:
>
> On Sat, 2021-03-27 at 10:19 +0200, Andy Shevchenko wrote:
> > On Saturday, March 27, 2021, Xiaofei Tan <tanxiaofei@huawei.com> wrote:
> >
> > > Fix some coding style issues reported by checkpatch.pl, including
> > > following types:
> > >
> > > WARNING: simple_strtol is obsolete, use kstrtol instead
> >
> >
> > And one more thing, the above message is bogus. Read what the comments in
> > the code says about use cases for simple_*() vs. kstrto*() ones.
> >
> > Joe?
>
> This check and message is nearly 10 years old and was appropriate for
> when it was implemented.
>
> kernel.h currently has:
>  * Use these functions if and only if you cannot use kstrto<foo>, because
>
> So the message could be changed to something like:
>
> WARNING: simple_strtol should be used only when kstrtol can not be used.

Fine with me, thanks!

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 07/15] ACPI: memhotplug: fix a coding style issue

[Xiaofei Tan]

Fix the following coding style issue reported by checkpatch.pl

WARNING: __initdata should be placed after acpi_no_memhotplug
FILE: drivers/acpi/acpi_memhotplug.c:326:
+static bool __initdata acpi_no_memhotplug;

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
---
 drivers/acpi/acpi_memhotplug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 8cc195c..5c5ed22 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -323,7 +323,7 @@ static void acpi_memory_device_remove(struct acpi_device *device)
 	acpi_memory_device_free(mem_device);
 }
 
-static bool __initdata acpi_no_memhotplug;
+static bool acpi_no_memhotplug __initdata;
 
 void __init acpi_memory_hotplug_init(void)
 {
-- 
2.8.1

[PATCH v2 08/15] ACPI: acpi_pad: fix a coding style issue

[Xiaofei Tan]

Fix the following coding style issue reported by checkpatch.pl

WARNING: Missing a blank line after declarations

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
---
 drivers/acpi/acpi_pad.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/acpi/acpi_pad.c b/drivers/acpi/acpi_pad.c
index b84ab72..df4adeb 100644
--- a/drivers/acpi/acpi_pad.c
+++ b/drivers/acpi/acpi_pad.c
@@ -128,6 +128,7 @@ static void round_robin_cpu(unsigned int tsk_index)
 static void exit_round_robin(unsigned int tsk_index)
 {
 	struct cpumask *pad_busy_cpus = to_cpumask(pad_busy_cpus_bits);
+
 	cpumask_clear_cpu(tsk_in_cpu[tsk_index], pad_busy_cpus);
 	tsk_in_cpu[tsk_index] = -1;
 }
@@ -265,6 +266,7 @@ static ssize_t rrtime_store(struct device *dev,
 	struct device_attribute *attr, const char *buf, size_t count)
 {
 	unsigned long num;
+
 	if (kstrtoul(buf, 0, &num))
 		return -EINVAL;
 	if (num < 1 || num >= 100)
@@ -286,6 +288,7 @@ static ssize_t idlepct_store(struct device *dev,
 	struct device_attribute *attr, const char *buf, size_t count)
 {
 	unsigned long num;
+
 	if (kstrtoul(buf, 0, &num))
 		return -EINVAL;
 	if (num < 1 || num >= 100)
@@ -307,6 +310,7 @@ static ssize_t idlecpus_store(struct device *dev,
 	struct device_attribute *attr, const char *buf, size_t count)
 {
 	unsigned long num;
+
 	if (kstrtoul(buf, 0, &num))
 		return -EINVAL;
 	mutex_lock(&isolated_cpus_lock);
-- 
2.8.1

[PATCH v2 09/15] ACPI: battery: fix some coding style issues

[Xiaofei Tan]

Fix some coding style issues reported by checkpatch.pl, including
following types:

WARNING: Block comments use * on subsequent lines
WARNING: Block comments use a trailing */ on a separate line
ERROR: code indent should use tabs where possible
WARNING: Missing a blank line after declarations
ERROR: spaces required around that '?' (ctx:WxV)
WARNING: Block comments should align the * on each line

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
---
 drivers/acpi/battery.c | 64 ++++++++++++++++++++++++++++++--------------------
 1 file changed, 38 insertions(+), 26 deletions(-)

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index b822f77..a0d8ead 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -74,16 +74,17 @@ enum {
 	ACPI_BATTERY_XINFO_PRESENT,
 	ACPI_BATTERY_QUIRK_PERCENTAGE_CAPACITY,
 	/* On Lenovo Thinkpad models from 2010 and 2011, the power unit
-	   switches between mWh and mAh depending on whether the system
-	   is running on battery or not.  When mAh is the unit, most
-	   reported values are incorrect and need to be adjusted by
-	   10000/design_voltage.  Verified on x201, t410, t410s, and x220.
-	   Pre-2010 and 2012 models appear to always report in mWh and
-	   are thus unaffected (tested with t42, t61, t500, x200, x300,
-	   and x230).  Also, in mid-2012 Lenovo issued a BIOS update for
-	   the 2011 models that fixes the issue (tested on x220 with a
-	   post-1.29 BIOS), but as of Nov. 2012, no such update is
-	   available for the 2010 models.  */
+	 * switches between mWh and mAh depending on whether the system
+	 * is running on battery or not.  When mAh is the unit, most
+	 * reported values are incorrect and need to be adjusted by
+	 * 10000/design_voltage.  Verified on x201, t410, t410s, and x220.
+	 * Pre-2010 and 2012 models appear to always report in mWh and
+	 * are thus unaffected (tested with t42, t61, t500, x200, x300,
+	 * and x230).  Also, in mid-2012 Lenovo issued a BIOS update for
+	 *  the 2011 models that fixes the issue (tested on x220 with a
+	 * post-1.29 BIOS), but as of Nov. 2012, no such update is
+	 * available for the 2010 models.
+	 */
 	ACPI_BATTERY_QUIRK_THINKPAD_MAH,
 	/* for batteries reporting current capacity with design capacity
 	 * on a full charge, but showing degradation in full charge cap.
@@ -372,8 +373,9 @@ static enum power_supply_property energy_battery_full_cap_broken_props[] = {
 };
 
 /* --------------------------------------------------------------------------
-                               Battery Management
-   -------------------------------------------------------------------------- */
+ *				Battery Management
+ * --------------------------------------------------------------------------
+ */
 struct acpi_offsets {
 	size_t offset;		/* offset inside struct acpi_sbs_battery */
 	u8 mode;		/* int or string? */
@@ -431,6 +433,7 @@ static int extract_package(struct acpi_battery *battery,
 {
 	int i;
 	union acpi_object *element;
+
 	if (package->type != ACPI_TYPE_PACKAGE)
 		return -EFAULT;
 	for (i = 0; i < num; ++i) {
@@ -439,6 +442,7 @@ static int extract_package(struct acpi_battery *battery,
 		element = &package->package.elements[i];
 		if (offsets[i].mode) {
 			u8 *ptr = (u8 *)battery + offsets[i].offset;
+
 			if (element->type == ACPI_TYPE_STRING ||
 			    element->type == ACPI_TYPE_BUFFER)
 				strncpy(ptr, element->string.pointer, 32);
@@ -497,10 +501,12 @@ static int extract_battery_info(const int use_bix,
 		    battery->design_capacity_warning *
 		    10000 / battery->design_voltage;
 		/* Curiously, design_capacity_low, unlike the rest of them,
-		   is correct.  */
+		 *  is correct.
+		 */
 		/* capacity_granularity_* equal 1 on the systems tested, so
-		   it's impossible to tell if they would need an adjustment
-		   or not if their values were higher.  */
+		 * it's impossible to tell if they would need an adjustment
+		 * or not if their values were higher.
+		 */
 	}
 	if (test_bit(ACPI_BATTERY_QUIRK_DEGRADED_FULL_CHARGE, &battery->flags) &&
 	    battery->capacity_now > battery->full_charge_capacity)
@@ -532,8 +538,8 @@ static int acpi_battery_get_info(struct acpi_battery *battery)
 		if (ACPI_FAILURE(status)) {
 			acpi_handle_info(battery->device->handle,
 					 "%s evaluation failed: %s\n",
-					 use_bix ?"_BIX":"_BIF",
-				         acpi_format_exception(status));
+					 use_bix ? "_BIX":"_BIF",
+					 acpi_format_exception(status));
 		} else {
 			result = extract_battery_info(use_bix,
 						      battery,
@@ -648,6 +654,7 @@ static ssize_t acpi_battery_alarm_show(struct device *dev,
 					char *buf)
 {
 	struct acpi_battery *battery = to_acpi_battery(dev_get_drvdata(dev));
+
 	return sprintf(buf, "%d\n", battery->alarm * 1000);
 }
 
@@ -657,6 +664,7 @@ static ssize_t acpi_battery_alarm_store(struct device *dev,
 {
 	unsigned long x;
 	struct acpi_battery *battery = to_acpi_battery(dev_get_drvdata(dev));
+
 	if (sscanf(buf, "%lu\n", &x) == 1)
 		battery->alarm = x/1000;
 	if (acpi_battery_present(battery))
@@ -743,7 +751,7 @@ EXPORT_SYMBOL_GPL(battery_hook_register);
  * This function gets called right after the battery sysfs
  * attributes have been added, so that the drivers that
  * define custom sysfs attributes can add their own.
-*/
+ */
 static void battery_hook_add_battery(struct acpi_battery *battery)
 {
 	struct acpi_battery_hook *hook_node, *tmp;
@@ -872,10 +880,12 @@ static void find_battery(const struct dmi_header *dm, void *private)
 {
 	struct acpi_battery *battery = (struct acpi_battery *)private;
 	/* Note: the hardcoded offsets below have been extracted from
-	   the source code of dmidecode.  */
+	 * the source code of dmidecode.
+	 */
 	if (dm->type == DMI_ENTRY_PORTABLE_BATTERY && dm->length >= 8) {
 		const u8 *dmi_data = (const u8 *)(dm + 1);
 		int dmi_capacity = get_unaligned((const u16 *)(dmi_data + 6));
+
 		if (dm->length >= 18)
 			dmi_capacity *= dmi_data[17];
 		if (battery->design_capacity * battery->design_voltage / 1000
@@ -917,6 +927,7 @@ static void acpi_battery_quirks(struct acpi_battery *battery)
 
 	if (battery->power_unit && dmi_name_in_vendors("LENOVO")) {
 		const char *s;
+
 		s = dmi_get_system_info(DMI_PRODUCT_VERSION);
 		if (s && !strncasecmp(s, "ThinkPad", 8)) {
 			dmi_walk(find_battery, battery);
@@ -1014,8 +1025,9 @@ static void acpi_battery_refresh(struct acpi_battery *battery)
 }
 
 /* --------------------------------------------------------------------------
-                                 Driver Interface
-   -------------------------------------------------------------------------- */
+ *				Driver Interface
+ * --------------------------------------------------------------------------
+ */
 
 static void acpi_battery_notify(struct acpi_device *device, u32 event)
 {
@@ -1026,11 +1038,11 @@ static void acpi_battery_notify(struct acpi_device *device, u32 event)
 		return;
 	old = battery->bat;
 	/*
-	* On Acer Aspire V5-573G notifications are sometimes triggered too
-	* early. For example, when AC is unplugged and notification is
-	* triggered, battery state is still reported as "Full", and changes to
-	* "Discharging" only after short delay, without any notification.
-	*/
+	 * On Acer Aspire V5-573G notifications are sometimes triggered too
+	 * early. For example, when AC is unplugged and notification is
+	 * triggered, battery state is still reported as "Full", and changes to
+	 * "Discharging" only after short delay, without any notification.
+	 */
 	if (battery_notification_delay_ms > 0)
 		msleep(battery_notification_delay_ms);
 	if (event == ACPI_BATTERY_NOTIFY_INFO)
-- 
2.8.1

[PATCH v2 10/15] ACPI: button: fix some coding style issues

[Xiaofei Tan]

Fix some coding style issues reported by checkpatch.pl, including
following types:

WARNING: Block comments use * on subsequent lines
WARNING: Block comments use a trailing */ on a separate line
ERROR: code indent should use tabs where possible

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
---
 drivers/acpi/button.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
index 85e5e03..c66db72 100644
--- a/drivers/acpi/button.c
+++ b/drivers/acpi/button.c
@@ -157,8 +157,9 @@ module_param(lid_report_interval, ulong, 0644);
 MODULE_PARM_DESC(lid_report_interval, "Interval (ms) between lid key events");
 
 /* --------------------------------------------------------------------------
-                              FS Interface (/proc)
-   -------------------------------------------------------------------------- */
+ *			FS Interface (/proc)
+ * --------------------------------------------------------------------------
+ */
 
 static struct proc_dir_entry *acpi_button_dir;
 static struct proc_dir_entry *acpi_lid_dir;
@@ -349,8 +350,9 @@ static int acpi_button_remove_fs(struct acpi_device *device)
 }
 
 /* --------------------------------------------------------------------------
-                                Driver Interface
-   -------------------------------------------------------------------------- */
+ *				Driver Interface
+ * --------------------------------------------------------------------------
+ */
 int acpi_lid_open(void)
 {
 	if (!lid_device)
-- 
2.8.1

[PATCH v2 11/15] ACPI: CPPC: fix some coding style issues

[Xiaofei Tan]

Fix some coding style issues reported by checkpatch.pl, including
following types:

WARNING: Missing a blank line after declarations
WARNING: unnecessary whitespace before a quoted newline
ERROR: spaces required around that '>='
ERROR: switch and case should be at the same indent

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
---
 drivers/acpi/cppc_acpi.c | 71 ++++++++++++++++++++++++------------------------
 1 file changed, 36 insertions(+), 35 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index ae53740..3dbaf47 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -326,6 +326,7 @@ static int send_pcc_cmd(int pcc_ss_id, u16 cmd)
 		if (unlikely(ret)) {
 			for_each_possible_cpu(i) {
 				struct cpc_desc *desc = per_cpu(cpc_desc_ptr, i);
+
 				if (!desc)
 					continue;
 
@@ -777,7 +778,7 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 			cpc_ptr->cpc_regs[i-2].type = ACPI_TYPE_BUFFER;
 			memcpy(&cpc_ptr->cpc_regs[i-2].cpc_entry.reg, gas_t, sizeof(*gas_t));
 		} else {
-			pr_debug("Err in entry:%d in CPC table of CPU:%d \n", i, pr->id);
+			pr_debug("Err in entry:%d in CPC table of CPU:%d\n", i, pr->id);
 			goto out_free;
 		}
 	}
@@ -867,7 +868,7 @@ void acpi_cppc_processor_exit(struct acpi_processor *pr)
 	void __iomem *addr;
 	int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, pr->id);
 
-	if (pcc_ss_id >=0 && pcc_data[pcc_ss_id]) {
+	if (pcc_ss_id >= 0 && pcc_data[pcc_ss_id]) {
 		if (pcc_data[pcc_ss_id]->pcc_channel_acquired) {
 			pcc_data[pcc_ss_id]->refcount--;
 			if (!pcc_data[pcc_ss_id]->refcount) {
@@ -954,22 +955,22 @@ static int cpc_read(int cpu, struct cpc_register_resource *reg_res, u64 *val)
 				val, reg->bit_width);
 
 	switch (reg->bit_width) {
-		case 8:
-			*val = readb_relaxed(vaddr);
-			break;
-		case 16:
-			*val = readw_relaxed(vaddr);
-			break;
-		case 32:
-			*val = readl_relaxed(vaddr);
-			break;
-		case 64:
-			*val = readq_relaxed(vaddr);
-			break;
-		default:
-			pr_debug("Error: Cannot read %u bit width from PCC for ss: %d\n",
-				 reg->bit_width, pcc_ss_id);
-			ret_val = -EFAULT;
+	case 8:
+		*val = readb_relaxed(vaddr);
+		break;
+	case 16:
+		*val = readw_relaxed(vaddr);
+		break;
+	case 32:
+		*val = readl_relaxed(vaddr);
+		break;
+	case 64:
+		*val = readq_relaxed(vaddr);
+		break;
+	default:
+		pr_debug("Error: Cannot read %u bit width from PCC for ss: %d\n",
+			 reg->bit_width, pcc_ss_id);
+		ret_val = -EFAULT;
 	}
 
 	return ret_val;
@@ -993,23 +994,23 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
 				val, reg->bit_width);
 
 	switch (reg->bit_width) {
-		case 8:
-			writeb_relaxed(val, vaddr);
-			break;
-		case 16:
-			writew_relaxed(val, vaddr);
-			break;
-		case 32:
-			writel_relaxed(val, vaddr);
-			break;
-		case 64:
-			writeq_relaxed(val, vaddr);
-			break;
-		default:
-			pr_debug("Error: Cannot write %u bit width to PCC for ss: %d\n",
-				 reg->bit_width, pcc_ss_id);
-			ret_val = -EFAULT;
-			break;
+	case 8:
+		writeb_relaxed(val, vaddr);
+		break;
+	case 16:
+		writew_relaxed(val, vaddr);
+		break;
+	case 32:
+		writel_relaxed(val, vaddr);
+		break;
+	case 64:
+		writeq_relaxed(val, vaddr);
+		break;
+	default:
+		pr_debug("Error: Cannot write %u bit width to PCC for ss: %d\n",
+			 reg->bit_width, pcc_ss_id);
+		ret_val = -EFAULT;
+		break;
 	}
 
 	return ret_val;
-- 
2.8.1

[PATCH v2 12/15] ACPI: custom_method: fix a coding style issue

[Xiaofei Tan]

Fix the following coding style issue reported by checkpatch.pl

ERROR: "foo * bar" should be "foo *bar"
FILE: drivers/acpi/custom_method.c:22:
+static ssize_t cm_write(struct file *file, const char __user * user_buf,

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
---
 drivers/acpi/custom_method.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/custom_method.c b/drivers/acpi/custom_method.c
index 7b54dc9..443fdf62 100644
--- a/drivers/acpi/custom_method.c
+++ b/drivers/acpi/custom_method.c
@@ -19,7 +19,7 @@ static struct dentry *cm_dentry;
 
 /* /sys/kernel/debug/acpi/custom_method */
 
-static ssize_t cm_write(struct file *file, const char __user * user_buf,
+static ssize_t cm_write(struct file *file, const char __user *user_buf,
 			size_t count, loff_t *ppos)
 {
 	static char *buf;
-- 
2.8.1

[PATCH v2 13/15] ACPI: PM: fix some coding style issues

[Xiaofei Tan]

Fix the following coding style issue reported by checkpatch.pl

WARNING: Missing a blank line after declarations

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
---
 drivers/acpi/device_pm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
index 0961537..16c0fe8 100644
--- a/drivers/acpi/device_pm.c
+++ b/drivers/acpi/device_pm.c
@@ -966,6 +966,7 @@ EXPORT_SYMBOL_GPL(acpi_dev_resume);
 int acpi_subsys_runtime_suspend(struct device *dev)
 {
 	int ret = pm_generic_runtime_suspend(dev);
+
 	return ret ? ret : acpi_dev_suspend(dev, true);
 }
 EXPORT_SYMBOL_GPL(acpi_subsys_runtime_suspend);
@@ -980,6 +981,7 @@ EXPORT_SYMBOL_GPL(acpi_subsys_runtime_suspend);
 int acpi_subsys_runtime_resume(struct device *dev)
 {
 	int ret = acpi_dev_resume(dev);
+
 	return ret ? ret : pm_generic_runtime_resume(dev);
 }
 EXPORT_SYMBOL_GPL(acpi_subsys_runtime_resume);
@@ -1171,6 +1173,7 @@ EXPORT_SYMBOL_GPL(acpi_subsys_freeze);
 int acpi_subsys_restore_early(struct device *dev)
 {
 	int ret = acpi_dev_resume(dev);
+
 	return ret ? ret : pm_generic_restore_early(dev);
 }
 EXPORT_SYMBOL_GPL(acpi_subsys_restore_early);
-- 
2.8.1

[PATCH v2 14/15] ACPI: sysfs: fix some coding style issues

[Xiaofei Tan]

Fix some coding style issues reported by checkpatch.pl, including
following types:

WARNING: Missing a blank line after declarations
WARNING: Block comments should align the * on each line
ERROR: open brace '{' following function definitions go on the next line

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
---
 drivers/acpi/device_sysfs.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
index da4ff2a..a07d4ad 100644
--- a/drivers/acpi/device_sysfs.c
+++ b/drivers/acpi/device_sysfs.c
@@ -73,6 +73,7 @@ static const struct sysfs_ops acpi_data_node_sysfs_ops = {
 static void acpi_data_node_release(struct kobject *kobj)
 {
 	struct acpi_data_node *dn = to_data_node(kobj);
+
 	complete(&dn->kobj_done);
 }
 
@@ -130,7 +131,7 @@ static void acpi_hide_nondev_subnodes(struct acpi_device_data *data)
  * Return: 0: no _HID and no _CID
  *         -EINVAL: output error
  *         -ENOMEM: output is truncated
-*/
+ */
 static int create_pnp_modalias(struct acpi_device *acpi_dev, char *modalias,
 			       int size)
 {
@@ -431,7 +432,8 @@ static DEVICE_ATTR_RO(path);
 /* sysfs file that shows description text from the ACPI _STR method */
 static ssize_t description_show(struct device *dev,
 				struct device_attribute *attr,
-				char *buf) {
+				char *buf)
+{
 	struct acpi_device *acpi_dev = to_acpi_device(dev);
 	int result;
 
@@ -456,7 +458,8 @@ static DEVICE_ATTR_RO(description);
 
 static ssize_t
 sun_show(struct device *dev, struct device_attribute *attr,
-	 char *buf) {
+	 char *buf)
+{
 	struct acpi_device *acpi_dev = to_acpi_device(dev);
 	acpi_status status;
 	unsigned long long sun;
@@ -471,7 +474,8 @@ static DEVICE_ATTR_RO(sun);
 
 static ssize_t
 hrv_show(struct device *dev, struct device_attribute *attr,
-	 char *buf) {
+	 char *buf)
+{
 	struct acpi_device *acpi_dev = to_acpi_device(dev);
 	acpi_status status;
 	unsigned long long hrv;
@@ -485,7 +489,8 @@ hrv_show(struct device *dev, struct device_attribute *attr,
 static DEVICE_ATTR_RO(hrv);
 
 static ssize_t status_show(struct device *dev, struct device_attribute *attr,
-				char *buf) {
+				char *buf)
+{
 	struct acpi_device *acpi_dev = to_acpi_device(dev);
 	acpi_status status;
 	unsigned long long sta;
-- 
2.8.1

[PATCH v2 15/15] ACPI: dock: fix some coding style issues

[Xiaofei Tan]

Fix some coding style issues reported by checkpatch.pl, including
following types:

WARNING: Missing a blank line after declarations
ERROR: spaces required around that ':'
WARNING: Statements should start on a tabstop

Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
---
 drivers/acpi/dock.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/dock.c b/drivers/acpi/dock.c
index 0937cea..7cf9215 100644
--- a/drivers/acpi/dock.c
+++ b/drivers/acpi/dock.c
@@ -271,6 +271,7 @@ static void hotplug_dock_devices(struct dock_station *ds, u32 event)
 
 		if (!acpi_device_enumerated(adev)) {
 			int ret = acpi_bus_scan(adev->handle);
+
 			if (ret)
 				dev_dbg(&adev->dev, "scan error %d\n", -ret);
 		}
@@ -502,6 +503,7 @@ static ssize_t flags_show(struct device *dev,
 			  struct device_attribute *attr, char *buf)
 {
 	struct dock_station *dock_station = dev->platform_data;
+
 	return snprintf(buf, PAGE_SIZE, "%d\n", dock_station->flags);
 
 }
@@ -523,7 +525,7 @@ static ssize_t undock_store(struct device *dev, struct device_attribute *attr,
 	begin_undock(dock_station);
 	ret = handle_eject_request(dock_station, ACPI_NOTIFY_EJECT_REQUEST);
 	acpi_scan_lock_release();
-	return ret ? ret: count;
+	return ret ? ret : count;
 }
 static DEVICE_ATTR_WO(undock);
 
@@ -535,10 +537,11 @@ static ssize_t uid_show(struct device *dev,
 {
 	unsigned long long lbuf;
 	struct dock_station *dock_station = dev->platform_data;
+
 	acpi_status status = acpi_evaluate_integer(dock_station->handle,
 					"_UID", NULL, &lbuf);
 	if (ACPI_FAILURE(status))
-	    return 0;
+		return 0;
 
 	return snprintf(buf, PAGE_SIZE, "%llx\n", lbuf);
 }
-- 
2.8.1

Re: [PATCH 00/15] acpi: fix some coding style issues

[Xiaofei Tan]

OK. thanks for reviewing this patch set.

On 2021/3/27 16:21, Andy Shevchenko wrote:
>
>
> On Saturday, March 27, 2021, Xiaofei Tan <tanxiaofei@huawei.com
> <mailto:tanxiaofei@huawei.com>> wrote:
>
>     Fix some coding style issues reported by checkpatch.pl
>     <http://checkpatch.pl>.
>
>
>
> NAK until it’s proven that you have tested each change on the real
> system (virtual more or less okay).
>
>
>
>     Differences from v1 to v2:
>     - Add subsystem and module name in the name of patch 05/15.
>     - Change to use more proper module name for some patch names.
>
>     Xiaofei Tan (15):
>       ACPI: APD: fix a block comment align issue
>       ACPI: processor: fix some coding style issues
>       ACPI: debug: fix some coding style issues
>       ACPI: table: replace __attribute__((packed)) by __packed
>       ACPI: ipmi: remove useless return statement for void function
>       ACPI: LPSS: fix some coding style issues
>       ACPI: memhotplug: fix a coding style issue
>       ACPI: acpi_pad: fix a coding style issue
>       ACPI: battery: fix some coding style issues
>       ACPI: button: fix some coding style issues
>       ACPI: CPPC: fix some coding style issues
>       ACPI: custom_method: fix a coding style issue
>       ACPI: PM: fix some coding style issues
>       ACPI: sysfs: fix some coding style issues
>       ACPI: dock: fix some coding style issues
>
>      drivers/acpi/acpi_apd.c        |  8 ++---
>      drivers/acpi/acpi_dbg.c        | 40 +++++++++++-------------
>      drivers/acpi/acpi_fpdt.c       |  6 ++--
>      drivers/acpi/acpi_ipmi.c       |  1 -
>      drivers/acpi/acpi_lpss.c       |  4 ++-
>      drivers/acpi/acpi_memhotplug.c |  2 +-
>      drivers/acpi/acpi_pad.c        |  4 +++
>      drivers/acpi/acpi_processor.c  | 18 +++--------
>      drivers/acpi/battery.c         | 64
>     +++++++++++++++++++++----------------
>      drivers/acpi/button.c          | 10 +++---
>      drivers/acpi/cppc_acpi.c       | 71
>     +++++++++++++++++++++---------------------
>      drivers/acpi/custom_method.c   |  2 +-
>      drivers/acpi/device_pm.c       |  3 ++
>      drivers/acpi/device_sysfs.c    | 15 ++++++---
>      drivers/acpi/dock.c            |  7 +++--
>      15 files changed, 138 insertions(+), 117 deletions(-)
>
>     --
>     2.8.1
>
>
>
> --
> With Best Regards,
> Andy Shevchenko
>
>