[PATCH v4 1/4] device property: Get rid of __PROPERTY_ENTRY_ARRAY_EL*SIZE*()

[PATCH v4 1/4] device property: Get rid of __PROPERTY_ENTRY_ARRAY_EL*SIZE*()

[Andy Shevchenko]

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@linux.intel.com>
Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
v4: added tag (Heikki)
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

[PATCH v4 2/4] device property: Move PROPERTY_ENTRY_BOOL() a bit down

[Andy Shevchenko]

Let's order ARRAY and non-ARRAY macros in the same way. The
PROPERTY_ENTRY_BOOL() is special, move it a bit down in the
code so it won't break ordering of the rest.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
v4: added tag (Heikki)
v3: no changes
v2: rebased on latest Linux Next
 include/linux/property.h | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/linux/property.h b/include/linux/property.h
index 0eab13a5c7df..37179e3abad5 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -330,6 +330,7 @@ struct property_entry {
 	__PROPERTY_ENTRY_ARRAY_LEN(_name_, u64_data, U64, _val_, _len_)
 #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_)		\
 (struct property_entry) {						\
 	.name = _name_,							\
@@ -348,7 +349,7 @@ struct property_entry {
 	PROPERTY_ENTRY_U64_ARRAY_LEN(_name_, _val_, ARRAY_SIZE(_val_))
 #define PROPERTY_ENTRY_STRING_ARRAY(_name_, _val_)			\
 	PROPERTY_ENTRY_STRING_ARRAY_LEN(_name_, _val_, ARRAY_SIZE(_val_))
-#define PROPERTY_ENTRY_REF_ARRAY(_name_, _val_)			\
+#define PROPERTY_ENTRY_REF_ARRAY(_name_, _val_)				\
 	PROPERTY_ENTRY_REF_ARRAY_LEN(_name_, _val_, ARRAY_SIZE(_val_))
 
 #define __PROPERTY_ENTRY_ELEMENT(_name_, _elem_, _Type_, _val_)		\
@@ -371,12 +372,6 @@ struct property_entry {
 #define PROPERTY_ENTRY_STRING(_name_, _val_)				\
 	__PROPERTY_ENTRY_ELEMENT(_name_, str, STRING, _val_)
 
-#define PROPERTY_ENTRY_BOOL(_name_)		\
-(struct property_entry) {			\
-	.name = _name_,				\
-	.is_inline = true,			\
-}
-
 #define PROPERTY_ENTRY_REF(_name_, _ref_, ...)				\
 (struct property_entry) {						\
 	.name = _name_,							\
@@ -385,9 +380,14 @@ struct property_entry {
 	{ .pointer = &SOFTWARE_NODE_REFERENCE(_ref_, ##__VA_ARGS__), },	\
 }
 
+#define PROPERTY_ENTRY_BOOL(_name_)		\
+(struct property_entry) {			\
+	.name = _name_,				\
+	.is_inline = true,			\
+}
+
 struct property_entry *
 property_entries_dup(const struct property_entry *properties);
-
 void property_entries_free(const struct property_entry *properties);
 
 bool device_dma_supported(const struct device *dev);
-- 
2.35.1

[PATCH v4 3/4] device property: Rename goto label to be more precise

[Andy Shevchenko]

In the fwnode_property_match_string() the goto label out has
an additional task. Rename the label to be more precise on
what is going to happen if goto it.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v4: new patch
 drivers/base/property.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index f7b5aa8fcf28..ed74083c179d 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -482,12 +482,13 @@ int fwnode_property_match_string(const struct fwnode_handle *fwnode,
 
 	ret = fwnode_property_read_string_array(fwnode, propname, values, nval);
 	if (ret < 0)
-		goto out;
+		goto out_free;
 
 	ret = match_string(values, nval, string);
 	if (ret < 0)
 		ret = -ENODATA;
-out:
+
+out_free:
 	kfree(values);
 	return ret;
 }
-- 
2.35.1

[PATCH v4 4/4] device property: Add a blank line in Kconfig of tests

[Andy Shevchenko]

Seems the blank line to separate entries in Kconfig was missing.
Add it.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
v4: added tag (Heikki)
v3: new patch
 drivers/base/test/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/base/test/Kconfig b/drivers/base/test/Kconfig
index 2f3fa31a948e..610a1ba7a467 100644
--- a/drivers/base/test/Kconfig
+++ b/drivers/base/test/Kconfig
@@ -8,6 +8,7 @@ config TEST_ASYNC_DRIVER_PROBE
 	  The module name will be test_async_driver_probe.ko
 
 	  If unsure say N.
+
 config DRIVER_PE_KUNIT_TEST
 	bool "KUnit Tests for property entry API" if !KUNIT_ALL_TESTS
 	depends on KUNIT=y
-- 
2.35.1

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

[Rafael J. Wysocki]

On Tue, Nov 22, 2022 at 2:35 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> 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@linux.intel.com>
> Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

The series in which this patch is included does not apply cleanly for me.

I guess it depends on the earlier material already in Greg's tree, so
I'm leaving it to Greg.

> ---
> v4: added tag (Heikki)
> 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
>

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

[Greg Kroah-Hartman]

On Wed, Nov 23, 2022 at 07:55:44PM +0100, Rafael J. Wysocki wrote:
> On Tue, Nov 22, 2022 at 2:35 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> 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@linux.intel.com>
> > Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> 
> The series in which this patch is included does not apply cleanly for me.
> 
> I guess it depends on the earlier material already in Greg's tree, so
> I'm leaving it to Greg.

Andy, did I miss this?

confused,

greg k-h

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

[Andy Shevchenko]

On Thu, Jan 19, 2023 at 04:40:53PM +0100, Greg Kroah-Hartman wrote:
> On Wed, Nov 23, 2022 at 07:55:44PM +0100, Rafael J. Wysocki wrote:
> > On Tue, Nov 22, 2022 at 2:35 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> 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@linux.intel.com>
> > > Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > 
> > The series in which this patch is included does not apply cleanly for me.
> > 
> > I guess it depends on the earlier material already in Greg's tree, so
> > I'm leaving it to Greg.
> 
> Andy, did I miss this?

Nope, thanks!
40eb28dc17f8 ("device property: Get rid of __PROPERTY_ENTRY_ARRAY_EL*SIZE*()")

-- 
With Best Regards,
Andy Shevchenko