Re: [PATCH v3 1/3] device property: Get rid of __PROPERTY_ENTRY_ARRAY_EL*SIZE*()

[Heikki Krogerus]

On Fri, Nov 11, 2022 at 05:46:19PM +0200, Andy Shevchenko wrote:
> First of all, _ELEMENT_SIZE() repeats existing sizeof_field() macro.
> Second, usage of _ARRAY_ELSIZE_LEN() adds unnecessary indirection
> to the data layout. It's more understandable when the data structure
> is placed explicitly. That said, get rid of those macros by replacing
> them with the existing helper and explicit data structure layout.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

For the series:

Acked-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>

> ---
> v3: fixed typo in PROPERTY_ENTRY_REF_ARRAY_LEN() impl (LKP)
> v2: rebased on latest Linux Next, fixed anon union assignment
>  include/linux/property.h | 34 ++++++++++++++--------------------
>  1 file changed, 14 insertions(+), 20 deletions(-)
> 
> diff --git a/include/linux/property.h b/include/linux/property.h
> index 5d840299146d..0eab13a5c7df 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -12,6 +12,7 @@
>  
>  #include <linux/bits.h>
>  #include <linux/fwnode.h>
> +#include <linux/stddef.h>
>  #include <linux/types.h>
>  
>  struct device;
> @@ -311,24 +312,14 @@ struct property_entry {
>   * crafted to avoid gcc-4.4.4's problems with initialization of anon unions
>   * and structs.
>   */
> -
> -#define __PROPERTY_ENTRY_ELEMENT_SIZE(_elem_)				\
> -	sizeof(((struct property_entry *)NULL)->value._elem_[0])
> -
> -#define __PROPERTY_ENTRY_ARRAY_ELSIZE_LEN(_name_, _elsize_, _Type_,	\
> -					  _val_, _len_)			\
> -(struct property_entry) {						\
> -	.name = _name_,							\
> -	.length = (_len_) * (_elsize_),					\
> -	.type = DEV_PROP_##_Type_,					\
> -	{ .pointer = _val_ },						\
> +#define __PROPERTY_ENTRY_ARRAY_LEN(_name_, _elem_, _Type_, _val_, _len_)		\
> +(struct property_entry) {								\
> +	.name = _name_,									\
> +	.length = (_len_) * sizeof_field(struct property_entry, value._elem_[0]),	\
> +	.type = DEV_PROP_##_Type_,							\
> +	{ .pointer = _val_ },								\
>  }
>  
> -#define __PROPERTY_ENTRY_ARRAY_LEN(_name_, _elem_, _Type_, _val_, _len_)\
> -	__PROPERTY_ENTRY_ARRAY_ELSIZE_LEN(_name_,			\
> -				__PROPERTY_ENTRY_ELEMENT_SIZE(_elem_),	\
> -				_Type_, _val_, _len_)
> -
>  #define PROPERTY_ENTRY_U8_ARRAY_LEN(_name_, _val_, _len_)		\
>  	__PROPERTY_ENTRY_ARRAY_LEN(_name_, u8_data, U8, _val_, _len_)
>  #define PROPERTY_ENTRY_U16_ARRAY_LEN(_name_, _val_, _len_)		\
> @@ -340,9 +331,12 @@ struct property_entry {
>  #define PROPERTY_ENTRY_STRING_ARRAY_LEN(_name_, _val_, _len_)		\
>  	__PROPERTY_ENTRY_ARRAY_LEN(_name_, str, STRING, _val_, _len_)
>  #define PROPERTY_ENTRY_REF_ARRAY_LEN(_name_, _val_, _len_)		\
> -	__PROPERTY_ENTRY_ARRAY_ELSIZE_LEN(_name_,			\
> -				sizeof(struct software_node_ref_args),	\
> -				REF, _val_, _len_)
> +(struct property_entry) {						\
> +	.name = _name_,							\
> +	.length = (_len_) * sizeof(struct software_node_ref_args),	\
> +	.type = DEV_PROP_REF,						\
> +	{ .pointer = _val_ },						\
> +}
>  
>  #define PROPERTY_ENTRY_U8_ARRAY(_name_, _val_)				\
>  	PROPERTY_ENTRY_U8_ARRAY_LEN(_name_, _val_, ARRAY_SIZE(_val_))
> @@ -360,7 +354,7 @@ struct property_entry {
>  #define __PROPERTY_ENTRY_ELEMENT(_name_, _elem_, _Type_, _val_)		\
>  (struct property_entry) {						\
>  	.name = _name_,							\
> -	.length = __PROPERTY_ENTRY_ELEMENT_SIZE(_elem_),		\
> +	.length = sizeof_field(struct property_entry, value._elem_[0]),	\
>  	.is_inline = true,						\
>  	.type = DEV_PROP_##_Type_,					\
>  	{ .value = { ._elem_[0] = _val_ } },				\
> -- 
> 2.35.1

-- 
heikki