diff mbox series

[v2,6/8] ACPI: property: Unify integer value reading functions

Message ID 20220520061148.21366-7-sakari.ailus@linux.intel.com
State New
Headers show
Series ACPI: Buffer property and reference as string support | expand

Commit Message

Sakari Ailus May 20, 2022, 6:11 a.m. UTC 
Unify functions reading ACPI property integer values into a single macro,
and call that macro to generate the functions for each bit depth.

Also use size_t for the counter instead of int.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/property.c | 79 +++++++++++------------------------------
 1 file changed, 20 insertions(+), 59 deletions(-)

Comments

Sakari Ailus May 20, 2022, 10:04 p.m. UTC | #1 
Hi Andy,

Thanks for the review.

On Fri, May 20, 2022 at 05:25:09PM +0300, Andy Shevchenko wrote:
> On Fri, May 20, 2022 at 09:11:46AM +0300, Sakari Ailus wrote:
> > Unify functions reading ACPI property integer values into a single macro,
> > and call that macro to generate the functions for each bit depth.
> > 
> > Also use size_t for the counter instead of int.
> 
> ...
> 
> > +#define DECLARE_ACPI_PROPERTY_COPY(bits)				\
> > +	static int							\
> > +	acpi_copy_property_array_u##bits(const union acpi_object *items, \
> 
> Personally I find much better if function templates in macros are not indented
> additionally.

I prefer it the way it is in this patch: indenting the macro has grounds as
it is part of the same pre-processor directive even if it's split on
multiple lines.

> 
> > +					 u##bits *val, size_t nval)	\
> > +	{								\
> > +		size_t i;						\
> > +									\
> > +		for (i = 0; i < nval; i++) {				\
> > +			if (items[i].type != ACPI_TYPE_INTEGER)		\
> > +				return -EPROTO;				\
> > +			if (items[i].integer.value > U##bits##_MAX)	\
> > +				return -EOVERFLOW;			\
> > +									\
> > +			val[i] = items[i].integer.value;		\
> > +		}							\
> > +		return 0;						\
> >  	}
> 
> On top of that, we use a minimum compiler that supports _Generic(). Why not to
> use it?

Good idea, I'll look into this. Perhaps we'll even get rid of having to
define multiple functions for this.
diff mbox series

Patch

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index a8e8a214a524f..8449479e18442 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -910,67 +910,28 @@  static int acpi_data_prop_read_single(const struct acpi_device_data *data,
 	return ret;
 }
 
-static int acpi_copy_property_array_u8(const union acpi_object *items, u8 *val,
-				       size_t nval)
-{
-	int i;
-
-	for (i = 0; i < nval; i++) {
-		if (items[i].type != ACPI_TYPE_INTEGER)
-			return -EPROTO;
-		if (items[i].integer.value > U8_MAX)
-			return -EOVERFLOW;
-
-		val[i] = items[i].integer.value;
-	}
-	return 0;
-}
-
-static int acpi_copy_property_array_u16(const union acpi_object *items,
-					u16 *val, size_t nval)
-{
-	int i;
-
-	for (i = 0; i < nval; i++) {
-		if (items[i].type != ACPI_TYPE_INTEGER)
-			return -EPROTO;
-		if (items[i].integer.value > U16_MAX)
-			return -EOVERFLOW;
-
-		val[i] = items[i].integer.value;
-	}
-	return 0;
-}
-
-static int acpi_copy_property_array_u32(const union acpi_object *items,
-					u32 *val, size_t nval)
-{
-	int i;
-
-	for (i = 0; i < nval; i++) {
-		if (items[i].type != ACPI_TYPE_INTEGER)
-			return -EPROTO;
-		if (items[i].integer.value > U32_MAX)
-			return -EOVERFLOW;
-
-		val[i] = items[i].integer.value;
+#define DECLARE_ACPI_PROPERTY_COPY(bits)				\
+	static int							\
+	acpi_copy_property_array_u##bits(const union acpi_object *items, \
+					 u##bits *val, size_t nval)	\
+	{								\
+		size_t i;						\
+									\
+		for (i = 0; i < nval; i++) {				\
+			if (items[i].type != ACPI_TYPE_INTEGER)		\
+				return -EPROTO;				\
+			if (items[i].integer.value > U##bits##_MAX)	\
+				return -EOVERFLOW;			\
+									\
+			val[i] = items[i].integer.value;		\
+		}							\
+		return 0;						\
 	}
-	return 0;
-}
 
-static int acpi_copy_property_array_u64(const union acpi_object *items,
-					u64 *val, size_t nval)
-{
-	int i;
-
-	for (i = 0; i < nval; i++) {
-		if (items[i].type != ACPI_TYPE_INTEGER)
-			return -EPROTO;
-
-		val[i] = items[i].integer.value;
-	}
-	return 0;
-}
+DECLARE_ACPI_PROPERTY_COPY(8)
+DECLARE_ACPI_PROPERTY_COPY(16)
+DECLARE_ACPI_PROPERTY_COPY(32)
+DECLARE_ACPI_PROPERTY_COPY(64)
 
 static int acpi_copy_property_array_string(const union acpi_object *items,
 					   char **val, size_t nval)