diff mbox series

[1/3] ACPICA: Introduce ACPI_ACCESS_BIT_WIDTH() macro

Message ID 20200212110540.83559-1-mika.westerberg@linux.intel.com (mailing list archive)
State Not Applicable
Headers show
Series [1/3] ACPICA: Introduce ACPI_ACCESS_BIT_WIDTH() macro | expand

Commit Message

Mika Westerberg Feb. 12, 2020, 11:05 a.m. UTC
Sometimes it is useful to find the access_width field value in bytes and
not in bits so add a helper that can be used for this purpose.

Suggested-by: Jean Delvare <jdelvare@suse.de>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 include/acpi/actypes.h | 1 +
 1 file changed, 1 insertion(+)

Comments

Jean Delvare Feb. 12, 2020, 11:52 a.m. UTC | #1
On Wed, 12 Feb 2020 14:05:38 +0300, Mika Westerberg wrote:
> Sometimes it is useful to find the access_width field value in bytes and
> not in bits so add a helper that can be used for this purpose.

s/ACPI_ACCESS_BIT_WIDTH/ACPI_ACCESS_BYTE_WIDTH/ in the subject.

> 
> Suggested-by: Jean Delvare <jdelvare@suse.de>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  include/acpi/actypes.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h
> index a2583c2bc054..77d40b02f62a 100644
> --- a/include/acpi/actypes.h
> +++ b/include/acpi/actypes.h
> @@ -537,6 +537,7 @@ typedef u64 acpi_integer;
>   * struct acpi_resource_generic_register.
>   */
>  #define ACPI_ACCESS_BIT_WIDTH(size)     (1 << ((size) + 2))
> +#define ACPI_ACCESS_BYTE_WIDTH(size)    (ACPI_ACCESS_BIT_WIDTH(size) / 8)

One of the points of having this macro being to avoid needless math,
I'd rather do:

#define ACPI_ACCESS_BYTE_WIDTH(size)    (1 << ((size) - 1))

Some compilers might be able to optimize it, but maybe not all, and I
see little point in giving the compiler more work anyway when it can be
easily avoided.

You may also want to replace "bit" by "bit or byte in the comment right
before the macros.

>  
>  /*******************************************************************************
>   *

Reviewed-by: Jean Delvare <jdelvare@suse.de>
Mika Westerberg Feb. 12, 2020, 12:08 p.m. UTC | #2
On Wed, Feb 12, 2020 at 12:52:44PM +0100, Jean Delvare wrote:
> On Wed, 12 Feb 2020 14:05:38 +0300, Mika Westerberg wrote:
> > Sometimes it is useful to find the access_width field value in bytes and
> > not in bits so add a helper that can be used for this purpose.
> 
> s/ACPI_ACCESS_BIT_WIDTH/ACPI_ACCESS_BYTE_WIDTH/ in the subject.

Indeed.

> > Suggested-by: Jean Delvare <jdelvare@suse.de>
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
> >  include/acpi/actypes.h | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h
> > index a2583c2bc054..77d40b02f62a 100644
> > --- a/include/acpi/actypes.h
> > +++ b/include/acpi/actypes.h
> > @@ -537,6 +537,7 @@ typedef u64 acpi_integer;
> >   * struct acpi_resource_generic_register.
> >   */
> >  #define ACPI_ACCESS_BIT_WIDTH(size)     (1 << ((size) + 2))
> > +#define ACPI_ACCESS_BYTE_WIDTH(size)    (ACPI_ACCESS_BIT_WIDTH(size) / 8)
> 
> One of the points of having this macro being to avoid needless math,
> I'd rather do:
> 
> #define ACPI_ACCESS_BYTE_WIDTH(size)    (1 << ((size) - 1))
> 
> Some compilers might be able to optimize it, but maybe not all, and I
> see little point in giving the compiler more work anyway when it can be
> easily avoided.
> 
> You may also want to replace "bit" by "bit or byte in the comment right
> before the macros.

OK, I'll do that.

> >  
> >  /*******************************************************************************
> >   *
> 
> Reviewed-by: Jean Delvare <jdelvare@suse.de>

Thanks!
diff mbox series

Patch

diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h
index a2583c2bc054..77d40b02f62a 100644
--- a/include/acpi/actypes.h
+++ b/include/acpi/actypes.h
@@ -537,6 +537,7 @@  typedef u64 acpi_integer;
  * struct acpi_resource_generic_register.
  */
 #define ACPI_ACCESS_BIT_WIDTH(size)     (1 << ((size) + 2))
+#define ACPI_ACCESS_BYTE_WIDTH(size)    (ACPI_ACCESS_BIT_WIDTH(size) / 8)
 
 /*******************************************************************************
  *