software node: add support for reference properties
v3 cover software node: add support for reference properties
These series implement "references" properties for software nodes as true properties, instead of managing them completely separately.
The first 10 patches are generic cleanups and consolidation and unification of the existing code; patch #11 implements PROPERTY_EMTRY_REF() and friends; patch #12 converts the user of references to the property syntax, and patch #13 removes the remains of references as entities that are managed separately.
Changes in v3:
- added various cleanups before implementing reference properties
Changes in v2:
- reworked code so that even single-entry reference properties are stored as arrays (i.e. the software_node_ref_args instances are not part of property_entry structure) to avoid size increase. From user's POV nothing is changed, one can still use PROPERTY_ENTRY_REF macro to define reference "inline".
- dropped unused DEV_PROP_MAX
- rebased on linux-next
Dmitry Torokhov (13):
software node: remove DEV_PROP_MAX
software node: clean up property_copy_string_array() software node: get rid of property_set_pointer() software node: simplify property_get_pointer() software node: remove property_entry_read_uNN_array functions software node: unify PROPERTY_ENTRY_XXX macros software node: simplify property_entry_read_string_array() software node: introduce PROPERTY_ENTRY_ARRAY_XXX_LEN() efi/apple-properties: use PROPERTY_ENTRY_U8_ARRAY_LEN software node: rename is_array to is_inline
software node: implement reference properties platform/x86: intel_cht_int33fe: use inline reference properties software node: remove separate handling of references
drivers/base/swnode.c | 243 +++++++---------------- drivers/firmware/efi/apple-properties.c | 8 +- drivers/platform/x86/intel_cht_int33fe.c | 81 ++++---- include/linux/property.h | 154 +++++++------- 4 files changed, 198 insertions(+), 288 deletions(-)
Signature · 1 line
2.23.0.187.g17f5b7556c-goog
PATCH v3 1/13 software node: remove DEV_PROP_MAX
This definition is not used anywhere, let's remove it.
--- include/linux/property.h | 1 - 1 file changed, 1 deletion(-)
include/linux/property.h +0 -1
@@ -22,7 +22,6 @@ enum dev_prop_type {DEV_PROP_U32 ,DEV_PROP_U64 , DEV_PROP_STRING, - DEV_PROP_MAX, }; enum dev_dma_attr {
Signature · 1 line
2.23.0.187.g17f5b7556c-goog
Re: [PATCH v3 01/13] software node: remove DEV_PROP_MAX
Quoted 2 lines · Dmitry Torokhov
This definition is not used anywhere, let's remove it.
Quoted 21 lines
Suggested-by Andy Shevchenko <[email protected]>Signed-off-by Dmitry Torokhov <[email protected]>--- include/linux/property.h | 1 - 1 file changed, 1 deletion(-)include/linux/property.h +0
-1@@ -22,7 +22,6 @@ enum dev_prop_type {DEV_PROP_U32 ,DEV_PROP_U64 , DEV_PROP_STRING, - DEV_PROP_MAX, }; enum dev_dma_attr {Signature · 1 line
2.23.0.187.g17f5b7556c-goog
Signature · 2 lines
With Best Regards, Andy Shevchenko
PATCH v3 2/13 software node: clean up property_copy_string_array()
In reply to Dmitry TorokhovBecause property_copy_string_array() stores the newly allocated pointer in the destination property, we have an awkward code in property_entry_copy_data() where we fetch the new pointer from dst.
Let's change property_copy_string_array() to return pointer and rely on the common path in property_entry_copy_data() to store it in destination structure.
--- drivers/base/swnode.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-)
drivers/base/swnode.c +8 -11
@@ -337,8 +337,8 @@ static void property_entry_free_data(const struct property_entry *p)kfree (p->name); } -static int property_copy_string_array (struct property_entry *dst, -const struct property_entry *src) +static const char *const * +property_copy_string_array (const struct property_entry *src) {const char **d;size_t nval = src->length /sizeof (*d);
@@ -346,7 +346,7 @@ static int property_copy_string_array(struct property_entry *dst, d =kcalloc (nval,sizeof (*d),GFP_KERNEL );if (!d) -return -ENOMEM ; +return NULL ;for (i =0 ; i < nval; i++) { d[i] =kstrdup (src->pointer.str[i],GFP_KERNEL );
@@ -354,12 +354,11 @@ static int property_copy_string_array(struct property_entry *dst,while (--i >=0 )kfree (d[i]);kfree (d); -return -ENOMEM ; +return NULL ; } } - dst->pointer.str = d; -return 0 ; +return d; } static int property_entry_copy_data(struct property_entry *dst,
@@ -367,17 +366,15 @@ static int property_entry_copy_data(struct property_entry *dst, {const void *pointer =property_get_pointer (src);const void *new; -int error;if (src->is_array) {if (!src->length)return -ENODATA ;if (src->type ==DEV_PROP_STRING ) { - error =property_copy_string_array (dst, src); -if (error) -return error; - new = dst->pointer.str; + new =property_copy_string_array (src); +if (!new) +return -ENOMEM ; }else { new =kmemdup (pointer, src->length,GFP_KERNEL );if (!new)
Signature · 1 line
2.23.0.187.g17f5b7556c-goog
PATCH v3 3/13 software node: get rid of property_set_pointer()
In reply to Dmitry TorokhovInstead of explicitly setting values of integer types when copying property entries lets just copy entire value union when processing non-array values.
When handling array values assign the pointer there using the newly introduced "raw" pointer union member. This allows us to remove property_set_pointer().
--- drivers/base/swnode.c | 50 +++++----------------------------------- include/linux/property.h | 1 + 2 files changed, 7 insertions(+), 44 deletions(-)
drivers/base/swnode.c +6 -44
@@ -103,45 +103,6 @@ property_entry_get(const struct property_entry *prop, const char *name)return NULL ; } -static void -property_set_pointer (struct property_entry *prop,const void *pointer) -{ -switch (prop->type) { -case DEV_PROP_U8 : -if (prop->is_array) - prop->pointer.u8_data = pointer; -else - prop->value.u8_data = *((u8 *)pointer); -break ; -case DEV_PROP_U16 : -if (prop->is_array) - prop->pointer.u16_data = pointer; -else - prop->value.u16_data = *((u16 *)pointer); -break ; -case DEV_PROP_U32 : -if (prop->is_array) - prop->pointer.u32_data = pointer; -else - prop->value.u32_data = *((u32 *)pointer); -break ; -case DEV_PROP_U64 : -if (prop->is_array) - prop->pointer.u64_data = pointer; -else - prop->value.u64_data = *((u64 *)pointer); -break ; -case DEV_PROP_STRING : -if (prop->is_array) - prop->pointer.str = pointer; -else - prop->value.str = pointer; -break ; -default : -break ; - } -} -static const void *property_get_pointer (const struct property_entry *prop) {switch (prop->type) {
@@ -380,20 +341,21 @@ static int property_entry_copy_data(struct property_entry *dst,if (!new)return -ENOMEM ; } + + dst->is_array = true; + dst->pointer.raw = new; }else if (src->type ==DEV_PROP_STRING ) { new =kstrdup (src->value.str,GFP_KERNEL );if (!new && src->value.str)return -ENOMEM ; + + dst->value.str = new; }else { - new = pointer; + dst->value = src->value; } dst->length = src->length; - dst->is_array = src->is_array; dst->type = src->type; - -property_set_pointer (dst, new); - dst->name =kstrdup (src->name,GFP_KERNEL );if (!dst->name) goto out_free_data;
include/linux/property.h +1 -0
@@ -239,6 +239,7 @@ struct property_entry {const u32 *u32_data;const u64 *u64_data;const char *const *str; +const void *raw; } pointer;union {u8 u8_data;
Signature · 1 line
2.23.0.187.g17f5b7556c-goog
Andy Shevchenko
reviewerRe: [PATCH v3 03/13] software node: get rid of property_set_pointer()
Quoted 5 lines · Dmitry Torokhov
Instead of explicitly setting values of integer types when copying property entries lets just copy entire value union when processing non-array values.
When handling array values assign the pointer there using the newly introduced "raw" pointer union member. This allows us to remove
property_set_pointer().
Is this reincarnation of 318a19718261?
Have you read 63dcc7090137?
From me NAK.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH v3 03/13] software node: get rid of property_set_pointer()
Quoted 9 lines · Andy Shevchenko
Quoted 5 lines · Dmitry Torokhov
Instead of explicitly setting values of integer types when copying property entries lets just copy entire value union when processing non-array values.
When handling array values assign the pointer there using the newly introduced "raw" pointer union member. This allows us to remove
property_set_pointer().Is this reincarnation of
318a19718261?
Have you read63dcc7090137?
Okay, I think if I squash this and the followup patch to property_get_data() then we'll only go through the "raw" pointer to get to the non-inline data and therefore we will not have the union aliasing issue.
The in-line values never change their type when storing/accessing.
Signature · 1 line
Dmitry
Andy Shevchenko
reviewerRe: [PATCH v3 03/13] software node: get rid of property_set_pointer()
Quoted 11 lines · Dmitry Torokhov
Quoted 9 lines · Andy Shevchenko
Quoted 5 lines · Dmitry Torokhov
Instead of explicitly setting values of integer types when copying property entries lets just copy entire value union when processing non-array values.
When handling array values assign the pointer there using the newly introduced "raw" pointer union member. This allows us to remove
property_set_pointer().Is this reincarnation of
318a19718261?
Have you read63dcc7090137?
Quoted 6 lines
Okay, I think if I squash this and the followup patch to
property_get_data()then we'll only go through the "raw" pointer to get to the non-inline data and therefore we will not have the union aliasing issue.The in-line values never change their type when storing/accessing.
It might work, though it prevents to do type checking at compile time. So, basically something like
struct obscure_things {
u8 *prop_array_val;
bla bla bla
};
struct property_entry entry;
struct obscure_things things;
...
entry.pointer.raw = &things;which shouldn't be possible.
I dunno what others think about your proposal.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH v3 03/13] software node: get rid of property_set_pointer()
Quoted 32 lines · Andy Shevchenko
Quoted 11 lines · Dmitry Torokhov
Quoted 9 lines · Andy Shevchenko
Quoted 5 lines · Dmitry Torokhov
Instead of explicitly setting values of integer types when copying property entries lets just copy entire value union when processing non-array values.
When handling array values assign the pointer there using the newly introduced "raw" pointer union member. This allows us to remove
property_set_pointer().Is this reincarnation of
318a19718261?
Have you read63dcc7090137?Quoted 6 lines
Okay, I think if I squash this and the followup patch to
property_get_data()then we'll only go through the "raw" pointer to get to the non-inline data and therefore we will not have the union aliasing issue.The in-line values never change their type when storing/accessing.
It might work, though it prevents to do type checking at compile time. So, basically something like
struct obscure_things { u8 *prop_array_val; bla bla bla }; struct property_entry entry; struct obscure_things things; ... entry.pointer.raw = &things;
Quoted 1 line
which shouldn't be possible.
I think type checking is a red herring as we still can't validate the type. I believe the answer here is to not allow external users to poke in property_entry and use PROPERTY_ENTRY_XXX macros to construct entires, as I have done for the Apple EFI driver.
Quoted 2 lines
I dunno what others think about your proposal.
Thanks.
Signature · 1 line
Dmitry
PATCH v3 4/13 software node: simplify property_get_pointer()
In reply to Dmitry TorokhovWe do not need to handle each data type separately, we can simply return either the raw pointer or pointer to values union.
--- drivers/base/swnode.c | 29 ++++++----------------------- 1 file changed, 6 insertions(+), 23 deletions(-)
drivers/base/swnode.c +6 -23
@@ -105,30 +105,13 @@ property_entry_get(const struct property_entry *prop, const char *name)static const void *property_get_pointer (const struct property_entry *prop) { -switch (prop->type) { -case DEV_PROP_U8 : -if (prop->is_array) -return prop->pointer.u8_data; -return &prop->value.u8_data; -case DEV_PROP_U16 : -if (prop->is_array) -return prop->pointer.u16_data; -return &prop->value.u16_data; -case DEV_PROP_U32 : -if (prop->is_array) -return prop->pointer.u32_data; -return &prop->value.u32_data; -case DEV_PROP_U64 : -if (prop->is_array) -return prop->pointer.u64_data; -return &prop->value.u64_data; -case DEV_PROP_STRING : -if (prop->is_array) -return prop->pointer.str; -return &prop->value.str; -default : +if (!prop->length)return NULL ; - } + +if (prop->is_array) +return prop->pointer.raw; + +return &prop->value; } static const void *property_entry_find(const struct property_entry *props,
Signature · 1 line
2.23.0.187.g17f5b7556c-goog
PATCH v3 5/13 software node: remove property_entry_read_uNN_array functions
In reply to Dmitry TorokhovThere is absolutely no reason to have them as we can handle it all nicely in property_entry_read_int_array().
--- drivers/base/swnode.c | 85 +++++++------------------------------------ 1 file changed, 14 insertions(+), 71 deletions(-)
drivers/base/swnode.c +14 -71
@@ -131,66 +131,6 @@ static const void *property_entry_find(const struct property_entry *props,return pointer; } -static int property_entry_read_u8_array (const struct property_entry *props, -const char *propname, -u8 *values,size_t nval) -{ -const void *pointer; -size_t length = nval *sizeof (*values); - - pointer =property_entry_find (props, propname, length); -if (IS_ERR (pointer)) -return PTR_ERR (pointer); - -memcpy (values, pointer, length); -return 0 ; -} - -static int property_entry_read_u16_array (const struct property_entry *props, -const char *propname, -u16 *values,size_t nval) -{ -const void *pointer; -size_t length = nval *sizeof (*values); - - pointer =property_entry_find (props, propname, length); -if (IS_ERR (pointer)) -return PTR_ERR (pointer); - -memcpy (values, pointer, length); -return 0 ; -} - -static int property_entry_read_u32_array (const struct property_entry *props, -const char *propname, -u32 *values,size_t nval) -{ -const void *pointer; -size_t length = nval *sizeof (*values); - - pointer =property_entry_find (props, propname, length); -if (IS_ERR (pointer)) -return PTR_ERR (pointer); - -memcpy (values, pointer, length); -return 0 ; -} - -static int property_entry_read_u64_array (const struct property_entry *props, -const char *propname, -u64 *values,size_t nval) -{ -const void *pointer; -size_t length = nval *sizeof (*values); - - pointer =property_entry_find (props, propname, length); -if (IS_ERR (pointer)) -return PTR_ERR (pointer); - -memcpy (values, pointer, length); -return 0 ; -} -static int property_entry_count_elems_of_size (const struct property_entry *props,const char *propname,size_t length)
@@ -209,21 +149,24 @@ static int property_entry_read_int_array(const struct property_entry *props, unsigned int elem_size, void *val, size_t nval) { + const void *pointer; +size_t length; +if (!val)return property_entry_count_elems_of_size (props, name, elem_size); -switch (elem_size) { -case sizeof (u8): -return property_entry_read_u8_array (props, name, val, nval); -case sizeof (u16): -return property_entry_read_u16_array (props, name, val, nval); -case sizeof (u32): -return property_entry_read_u32_array (props, name, val, nval); -case sizeof (u64): -return property_entry_read_u64_array (props, name, val, nval); - } -return -ENXIO ; +if (!is_power_of_2 (elem_size) || elem_size >sizeof (u64)) +return -ENXIO ; + + length = nval * elem_size; + + pointer =property_entry_find (props, name, length); +if (IS_ERR (pointer)) +return PTR_ERR (pointer); + +memcpy (val, pointer, length); +return 0 ; } static int property_entry_read_string_array(const struct property_entry *props,
Signature · 1 line
2.23.0.187.g17f5b7556c-goog
PATCH v3 6/13 software node: unify PROPERTY_ENTRY_XXX macros
In reply to Dmitry TorokhovWe can unify string properties initializer macros with integer initializers.
--- include/linux/property.h | 70 +++++++++++++++++----------------------- 1 file changed, 30 insertions(+), 40 deletions(-)
include/linux/property.h +30 -40
@@ -257,57 +257,47 @@ struct property_entry {* and structs. */ -#define PROPERTY_ENTRY_INTEGER_ARRAY (_name_, _type_, _Type_, _val_) \ +#define PROPERTY_ENTRY_ELSIZE (_elem_) \ + sizeof(*(((struct property_entry *)NULL)->pointer._elem_)) + +#define PROPERTY_ENTRY_ARRAY (_name_, _elem_, _Type_, _val_) \ (struct property_entry) { \ .name = _name_, \ - .length = ARRAY_SIZE(_val_) * sizeof(_type_), \ + .length = ARRAY_SIZE(_val_) * PROPERTY_ENTRY_ELSIZE(_elem_), \ .is_array = true, \ .type = DEV_PROP_##_Type_, \ - { .pointer = { ._type_##_data = _val_ } }, \ + .pointer._elem_ = _val_, \ } -#define PROPERTY_ENTRY_U8_ARRAY (_name_, _val_) \ - PROPERTY_ENTRY_INTEGER_ARRAY(_name_, u8, U8, _val_) -#define PROPERTY_ENTRY_U16_ARRAY (_name_, _val_) \ - PROPERTY_ENTRY_INTEGER_ARRAY(_name_, u16, U16, _val_) -#define PROPERTY_ENTRY_U32_ARRAY (_name_, _val_) \ - PROPERTY_ENTRY_INTEGER_ARRAY(_name_, u32, U32, _val_) -#define PROPERTY_ENTRY_U64_ARRAY (_name_, _val_) \ - PROPERTY_ENTRY_INTEGER_ARRAY(_name_, u64, U64, _val_) - -#define PROPERTY_ENTRY_STRING_ARRAY (_name_, _val_) \ +#define PROPERTY_ENTRY_U8_ARRAY (_name_, _val_) \ + PROPERTY_ENTRY_ARRAY(_name_, u8_data, U8, _val_) +#define PROPERTY_ENTRY_U16_ARRAY (_name_, _val_) \ + PROPERTY_ENTRY_ARRAY(_name_, u16_data, U16, _val_) +#define PROPERTY_ENTRY_U32_ARRAY (_name_, _val_) \ + PROPERTY_ENTRY_ARRAY(_name_, u32_data, U32, _val_) +#define PROPERTY_ENTRY_U64_ARRAY (_name_, _val_) \ + PROPERTY_ENTRY_ARRAY(_name_, u64_data, U64, _val_) +#define PROPERTY_ENTRY_STRING_ARRAY (_name_, _val_) \ + PROPERTY_ENTRY_ARRAY(_name_, str, STRING, _val_) + +#define PROPERTY_ENTRY_ELEMENT (_name_, _elem_, _Type_, _val_) \ (struct property_entry) { \ .name = _name_, \ - .length = ARRAY_SIZE(_val_) * sizeof(const char *), \ - .is_array = true, \ - .type = DEV_PROP_STRING, \ - { .pointer = { .str = _val_ } }, \ -} - -#define PROPERTY_ENTRY_INTEGER (_name_, _type_, _Type_, _val_) \ -(struct property_entry) { \ - .name = _name_, \ - .length = sizeof(_type_), \ + .length = PROPERTY_ENTRY_ELSIZE(_elem_), \ .type = DEV_PROP_##_Type_, \ - { .value = { ._type_##_data = _val_ } }, \ + .value._elem_ = _val_, \ } -#define PROPERTY_ENTRY_U8 (_name_, _val_) \ - PROPERTY_ENTRY_INTEGER(_name_, u8, U8, _val_) -#define PROPERTY_ENTRY_U16 (_name_, _val_) \ - PROPERTY_ENTRY_INTEGER(_name_, u16, U16, _val_) -#define PROPERTY_ENTRY_U32 (_name_, _val_) \ - PROPERTY_ENTRY_INTEGER(_name_, u32, U32, _val_) -#define PROPERTY_ENTRY_U64 (_name_, _val_) \ - PROPERTY_ENTRY_INTEGER(_name_, u64, U64, _val_) - -#define PROPERTY_ENTRY_STRING (_name_, _val_) \ -(struct property_entry) { \ - .name = _name_, \ - .length = sizeof(const char *), \ - .type = DEV_PROP_STRING, \ - { .value = { .str = _val_ } }, \ -} +#define PROPERTY_ENTRY_U8 (_name_, _val_) \ + PROPERTY_ENTRY_ELEMENT(_name_, u8_data, U8, _val_) +#define PROPERTY_ENTRY_U16 (_name_, _val_) \ + PROPERTY_ENTRY_ELEMENT(_name_, u16_data, U16, _val_) +#define PROPERTY_ENTRY_U32 (_name_, _val_) \ + PROPERTY_ENTRY_ELEMENT(_name_, u32_data, U32, _val_) +#define PROPERTY_ENTRY_U64 (_name_, _val_) \ + PROPERTY_ENTRY_ELEMENT(_name_, u64_data, U64, _val_) +#define PROPERTY_ENTRY_STRING (_name_, _val_) \ + PROPERTY_ENTRY_ELEMENT(_name_, str, STRING, _val_)#define PROPERTY_ENTRY_BOOL (_name_) \ (struct property_entry) { \
Signature · 1 line
2.23.0.187.g17f5b7556c-goog
PATCH v3 7/13 software node: simplify property_entry_read_string_array()
In reply to Dmitry TorokhovThere is no need to treat string arrays and single strings separately, we can go exclusively by the element length in relation to data type size.
--- drivers/base/swnode.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-)
drivers/base/swnode.c +7 -14
@@ -173,28 +173,21 @@ static int property_entry_read_string_array(const struct property_entry *props,const char *propname, const char **strings, size_t nval) { - const struct property_entry *prop; const void *pointer; -size_t array_len, length; +size_t length; +int array_len;/* Find out the array length. */ - prop =property_entry_get (props, propname); -if (!prop) -return -EINVAL ; - -if (prop->is_array) -/* Find the length of an array. */ - array_len =property_entry_count_elems_of_size (props, propname, -sizeof (const char *)); -else -/* The array length for a non-array string property is 1. */ - array_len =1 ; + array_len =property_entry_count_elems_of_size (props, propname, +sizeof (const char *)); +if (array_len <0 ) +return array_len;/* Return how many there are if strings is NULL. */ if (!strings)return array_len; - array_len =min (nval, array_len); + array_len =min_t (size_t, nval, array_len); length = array_len *sizeof (*strings); pointer =property_entry_find (props, propname, length);
Signature · 1 line
2.23.0.187.g17f5b7556c-goog
PATCH v3 8/13 software node: introduce PROPERTY_ENTRY_ARRAY_XXX_LEN()
In reply to Dmitry TorokhovSometimes we want to initialize property entry array from a regular pointer, when we can't determine length automatically via ARRAY_SIZE. Let's introduce PROPERTY_ENTRY_ARRAY_XXX_LEN macros that take explicit "len" argument.
--- include/linux/property.h | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
include/linux/property.h +19 -0
@@ -280,6 +280,25 @@ struct property_entry {#define PROPERTY_ENTRY_STRING_ARRAY (_name_, _val_) \ PROPERTY_ENTRY_ARRAY(_name_, str, STRING, _val_) +#define PROPERTY_ENTRY_ARRAY_LEN (_name_, _elem_, _Type_, _val_, _len_) \ +(struct property_entry) { \ + .name = _name_, \ + .length = (_len_) * PROPERTY_ENTRY_ELSIZE(_elem_), \ + .type = DEV_PROP_##_Type_, \ + .pointer._elem_ = _val_, \ +} + +#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_) \ + PROPERTY_ENTRY_ARRAY_LEN(_name_, u16_data, U16, _val_, _len_) +#define PROPERTY_ENTRY_U32_ARRAY_LEN (_name_, _val_, _len_) \ + PROPERTY_ENTRY_ARRAY_LEN(_name_, u32_data, U32, _val_, _len_) +#define PROPERTY_ENTRY_U64_ARRAY_LEN (_name_, _val_, _len_) \ + 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_ELEMENT (_name_, _elem_, _Type_, _val_) \ (struct property_entry) { \ .name = _name_, \
Signature · 1 line
2.23.0.187.g17f5b7556c-goog
PATCH v3 9/13 efi/apple-properties: use PROPERTY_ENTRY_U8_ARRAY_LEN
In reply to Dmitry TorokhovLet's switch to using PROPERTY_ENTRY_U8_ARRAY_LEN() to initialize property entries instead of poking into the structure directly.
--- drivers/firmware/efi/apple-properties.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
drivers/firmware/efi/apple-properties.c +3 -5
@@ -85,11 +85,9 @@ static void __init unmarshal_key_value_pairs(struct dev_header *dev_header,ucs2_as_utf8 (key, ptr +sizeof (key_len), key_len -sizeof (key_len)); - entry[i].name = key; - entry[i].length = val_len -sizeof (val_len); - entry[i].is_array = !!entry[i].length; - entry[i].type =DEV_PROP_U8 ; - entry[i].pointer.u8_data = ptr + key_len +sizeof (val_len); + entry[i] =PROPERTY_ENTRY_U8_ARRAY_LEN (key, + ptr + key_len +sizeof (val_len), + val_len -sizeof (val_len));if (dump_properties) {dev_info (dev,"property: %s\n" , entry[i].name);
Signature · 1 line
2.23.0.187.g17f5b7556c-goog
PATCH v3 10/13 software node: rename is_array to is_inline
In reply to Dmitry TorokhovWe do not need a special flag to know if we are dealing with an array, as we can get that data from ration between element length and the data size, however we do need a flag to know whether the data is stored directly inside property_entry or separately.
--- drivers/base/swnode.c | 9 +++++---- include/linux/property.h | 7 ++++--- 2 files changed, 9 insertions(+), 7 deletions(-)
drivers/base/swnode.c +5 -4
@@ -108,7 +108,7 @@ static const void *property_get_pointer(const struct property_entry *prop)if (!prop->length)return NULL ; -if (prop->is_array) +if (!prop->is_inline)return prop->pointer.raw;return &prop->value;
@@ -204,7 +204,7 @@ static void property_entry_free_data(const struct property_entry *p)const void *pointer =property_get_pointer (p);size_t i, nval; -if (p->is_array) { +if (!p->is_inline) {if (p->type ==DEV_PROP_STRING && p->pointer.str) { nval = p->length /sizeof (const char *);for (i =0 ; i < nval; i++)
@@ -247,7 +247,7 @@ static int property_entry_copy_data(struct property_entry *dst,const void *pointer =property_get_pointer (src);const void *new; -if (src->is_array) { +if (!src->is_inline) {if (!src->length)return -ENODATA ;
@@ -261,15 +261,16 @@ static int property_entry_copy_data(struct property_entry *dst,return -ENOMEM ; } - dst->is_array = true; dst->pointer.raw = new; }else if (src->type ==DEV_PROP_STRING ) { new =kstrdup (src->value.str,GFP_KERNEL );if (!new && src->value.str)return -ENOMEM ; + dst->is_inline = true; dst->value.str = new; }else { + dst->is_inline = true; dst->value = src->value; }
include/linux/property.h +4 -3
@@ -222,7 +222,8 @@ static inline int fwnode_property_count_u64(const struct fwnode_handle *fwnode,* struct property_entry - "Built-in" device property representation. * @name: Name of the property. * @length: Length of data making up the value. -* @is_array: True when the property is an array. +* @is_inline: True when the property value is stored directly in +* &struct property_entry instance. * @type: Type of the data in unions. * @pointer: Pointer to the property (an array of items of the given type). * @value: Value of the property (when it is a single item of the given type).
@@ -230,7 +231,7 @@ static inline int fwnode_property_count_u64(const struct fwnode_handle *fwnode, struct property_entry { const char *name; size_t length; - bool is_array; + bool is_inline; enum dev_prop_type type; union { union {
@@ -264,7 +265,6 @@ struct property_entry { (struct property_entry ) { \ .name = _name_, \ .length =ARRAY_SIZE (_val_) *PROPERTY_ENTRY_ELSIZE (_elem_), \ - .is_array = true, \ .type = DEV_PROP_##_Type_, \ .pointer._elem_ = _val_, \ }
@@ -304,6 +304,7 @@ struct property_entry { .name = _name_, \ .length = PROPERTY_ENTRY_ELSIZE(_elem_), \ .type = DEV_PROP_##_Type_, \ + .is_inline = true, \ .value._elem_ = _val_, \ }
Signature · 1 line
2.23.0.187.g17f5b7556c-goog
PATCH v3 11/13 software node: implement reference properties
In reply to Dmitry TorokhovIt is possible to store references to software nodes in the same fashion as other static properties, so that users do not need to define separate structures:
static const struct software_node gpio_bank_b_node = {
.name = "B",
};static const struct property_entry simone_key_enter_props[] = {
PROPERTY_ENTRY_U32("linux,code", KEY_ENTER),
PROPERTY_ENTRY_STRING("label", "enter"),
PROPERTY_ENTRY_REF("gpios", &gpio_bank_b_node, 123, GPIO_ACTIVE_LOW),
{ }
};--- drivers/base/swnode.c | 46 +++++++++++++++++++++++++++++++++------- include/linux/property.h | 46 ++++++++++++++++++++++++++++------------ 2 files changed, 70 insertions(+), 22 deletions(-)
drivers/base/swnode.c +38 -8
@@ -262,6 +262,12 @@ static int property_entry_copy_data(struct property_entry *dst, } dst->pointer.raw = new; + }else if (src->type ==DEV_PROP_REF ) { +/* +* Reference properties are never stored inline as +* they are too big. +*/ +return -EINVAL ; }else if (src->type ==DEV_PROP_STRING ) { new =kstrdup (src->value.str,GFP_KERNEL );if (!new && src->value.str)
@@ -447,21 +453,45 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode, {struct swnode *swnode =to_swnode (fwnode);const struct software_node_reference *ref; +const struct software_node_ref_args *ref_args;const struct property_entry *prop;struct fwnode_handle *refnode;int i; -if (!swnode || !swnode->node->references) +if (!swnode)return -ENOENT ; -for (ref = swnode->node->references; ref->name; ref++) -if (!strcmp (ref->name, propname)) -break ; + prop =property_entry_get (swnode->node->properties, propname); +if (prop) { +if (prop->type !=DEV_PROP_REF ) +return -EINVAL ; -if (!ref->name || index > (ref->nrefs -1 )) -return -ENOENT ; +/* +* We expect that references are never stored inline, even +* single ones, as they are too big. +*/ +if (prop->is_inline) +return -EINVAL ; + +if (index *sizeof (*ref_args) >= prop->length) +return -ENOENT ; + + ref_args = &prop->pointer.ref[index]; + }else { +if (!swnode->node->references) +return -ENOENT ; + +for (ref = swnode->node->references; ref->name; ref++) +if (!strcmp (ref->name, propname)) +break ; + +if (!ref->name || index > (ref->nrefs -1 )) +return -ENOENT ; + + ref_args = &ref->refs[index]; + } - refnode =software_node_fwnode (ref->refs[index].node); + refnode =software_node_fwnode (ref_args->node);if (!refnode)return -ENOENT ;
@@ -480,7 +510,7 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode, args->nargs = nargs;for (i =0 ; i < nargs; i++) - args->args[i] = ref->refs[index].args[i]; + args->args[i] = ref_args->args[i];return 0 ; }
include/linux/property.h +32 -14
88 lines not on this page · raw message
Signature · 1 line
2.23.0.187.g17f5b7556c-goog
PATCH v3 12/13 platform/x86: intel_cht_int33fe: use inline reference properties
In reply to Dmitry TorokhovNow that static device properties allow defining reference properties together with all other types of properties, instead of managing them separately, let's adjust the driver.
--- drivers/platform/x86/intel_cht_int33fe.c | 81 ++++++++++++------------ 1 file changed, 41 insertions(+), 40 deletions(-)
drivers/platform/x86/intel_cht_int33fe.c +41 -40
@@ -46,30 +46,6 @@ struct cht_int33fe_data {struct fwnode_handle *dp; }; -static const struct software_node nodes[]; - -static const struct software_node_ref_args pi3usb30532_ref = { - &nodes[INT33FE_NODE_PI3USB30532 ] -}; - -static const struct software_node_ref_args dp_ref = { - &nodes[INT33FE_NODE_DISPLAYPORT ] -}; - -static struct software_node_ref_args mux_ref; - -static const struct software_node_reference usb_connector_refs[] = { - {"orientation-switch" ,1 , &pi3usb30532_ref}, - {"mode-switch" ,1 , &pi3usb30532_ref}, - {"displayport" ,1 , &dp_ref}, - { } -}; - -static const struct software_node_reference fusb302_refs[] = { - {"usb-role-switch" ,1 , &mux_ref}, - { } -}; -/* * Grrr I severly dislike buggy BIOS-es. At least one BIOS enumerates * the max17047 both through the INT33FE ACPI device (it is right there
@@ -105,8 +81,18 @@ static const struct property_entry max17047_props[] = { { } }; +/* +* We are not using inline property here because those are constant, +* and we need to adjust this one at runtime to point to real +* software node. +*/ +static struct software_node_ref_args fusb302_mux_refs[] = { + { .node =NULL }, +}; +static const struct property_entry fusb302_props[] = {PROPERTY_ENTRY_STRING ("linux,extcon-name" ,"cht_wcove_pwrsrc" ), +PROPERTY_ENTRY_REF_ARRAY ("usb-role-switch" , fusb302_mux_refs), { } };
@@ -122,6 +108,8 @@ static const u32 snk_pdo[] = { PDO_VAR(5000, 12000, 3000), }; +static const struct software_node nodes[]; +static const struct property_entry usb_connector_props[] = {PROPERTY_ENTRY_STRING ("data-role" ,"dual" ),PROPERTY_ENTRY_STRING ("power-role" ,"dual" ),
@@ -129,15 +117,21 @@ static const struct property_entry usb_connector_props[] = {PROPERTY_ENTRY_U32_ARRAY ("source-pdos" , src_pdo),PROPERTY_ENTRY_U32_ARRAY ("sink-pdos" , snk_pdo),PROPERTY_ENTRY_U32 ("op-sink-microwatt" ,2500000 ), +PROPERTY_ENTRY_REF ("orientation-switch" , + &nodes[INT33FE_NODE_PI3USB30532 ]), +PROPERTY_ENTRY_REF ("mode-switch" , + &nodes[INT33FE_NODE_PI3USB30532 ]), +PROPERTY_ENTRY_REF ("displayport" , + &nodes[INT33FE_NODE_DISPLAYPORT]), { } };static const struct software_node nodes[] = { - {"fusb302" ,NULL , fusb302_props, fusb302_refs }, + {"fusb302" ,NULL , fusb302_props }, {"max17047" ,NULL , max17047_props }, {"pi3usb30532" }, {"displayport" }, - {"connector" , &nodes[0 ], usb_connector_props, usb_connector_refs }, + {"connector" , &nodes[0 ], usb_connector_props }, { } };
@@ -173,9 +167,10 @@ static void cht_int33fe_remove_nodes(struct cht_int33fe_data *data) {software_node_unregister_nodes (nodes); -if (mux_ref.node) { -fwnode_handle_put (software_node_fwnode (mux_ref.node)); - mux_ref.node =NULL ; +if (fusb302_mux_refs[0 ].node) { +fwnode_handle_put ( +software_node_fwnode (fusb302_mux_refs[0 ].node)); + fusb302_mux_refs[0 ].node =NULL ; }if (data->dp) {
@@ -187,25 +182,31 @@ static void cht_int33fe_remove_nodes(struct cht_int33fe_data *data)static int cht_int33fe_add_nodes (struct cht_int33fe_data *data) { +const struct software_node *mux_ref_node;int ret; - ret =software_node_register_nodes (nodes); -if (ret) -return ret; - -/* The devices that are not created in this driver need extra steps. */ -/* * There is no ACPI device node for the USB role mux, so we need to wait * until the mux driver has created software node for the mux device. * It means we depend on the mux driver. This function will return * -EPROBE_DEFER until the mux device is registered. */ - mux_ref.node =software_node_find_by_name (NULL ,"intel-xhci-usb-sw" ); -if (!mux_ref.node) { - ret = -EPROBE_DEFER ; - goto err_remove_nodes; - } + mux_ref_node =software_node_find_by_name (NULL ,"intel-xhci-usb-sw" ); +if (!mux_ref_node) +return -EPROBE_DEFER ; + +/* +* Update node used in "usb-role-switch" property. Note that we +* rely on software_node_register_nodes() to use the original +* instance of properties instead of copying them. +*/ + fusb302_mux_refs[0 ].node = mux_ref_node; + + ret =software_node_register_nodes (nodes); +if (ret) +return ret; + +/* The devices that are not created in this driver need extra steps. */ /* * The DP connector does have ACPI device node. In this case we can just
Signature · 1 line
2.23.0.187.g17f5b7556c-goog
PATCH v3 13/13 software node: remove separate handling of references
In reply to Dmitry TorokhovNow that all users of references have moved to reference properties, we can remove separate handling of references.
--- drivers/base/swnode.c | 44 +++++++++++++++------------------------- include/linux/property.h | 14 ------------- 2 files changed, 16 insertions(+), 42 deletions(-)
drivers/base/swnode.c +16 -28
@@ -452,8 +452,7 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode, struct fwnode_reference_args *args) {struct swnode *swnode =to_swnode (fwnode); -const struct software_node_reference *ref; -const struct software_node_ref_args *ref_args; +const struct software_node_ref_args *ref;const struct property_entry *prop;struct fwnode_handle *refnode;int i;
@@ -462,36 +461,25 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode,return -ENOENT ; prop =property_entry_get (swnode->node->properties, propname); -if (prop) { -if (prop->type !=DEV_PROP_REF ) -return -EINVAL ; - -/* -* We expect that references are never stored inline, even -* single ones, as they are too big. -*/ -if (prop->is_inline) -return -EINVAL ; - -if (index *sizeof (*ref_args) >= prop->length) -return -ENOENT ; +if (!prop) +return -ENOENT ; - ref_args = &prop->pointer.ref[index]; - }else { -if (!swnode->node->references) -return -ENOENT ; +if (prop->type !=DEV_PROP_REF ) +return -EINVAL ; -for (ref = swnode->node->references; ref->name; ref++) -if (!strcmp (ref->name, propname)) -break ; +/* +* We expect that references are never stored inline, even +* single ones, as they are too big. +*/ +if (prop->is_inline) +return -EINVAL ; -if (!ref->name || index > (ref->nrefs -1 )) -return -ENOENT ; +if (index *sizeof (*ref) >= prop->length) +return -ENOENT ; - ref_args = &ref->refs[index]; - } + ref = &prop->pointer.ref[index]; - refnode =software_node_fwnode (ref_args->node); + refnode =software_node_fwnode (ref->node);if (!refnode)return -ENOENT ;
@@ -510,7 +498,7 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode, args->nargs = nargs;for (i =0 ; i < nargs; i++) - args->args[i] = ref_args->args[i]; + args->args[i] = ref->args[i];return 0 ; }
include/linux/property.h +0 -14
@@ -419,30 +419,16 @@ int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,/* -------------------------------------------------------------------------- */ /* Software fwnode support - when HW description is incomplete or missing */ -/** -* struct software_node_reference - Named software node reference property -* @name: Name of the property -* @nrefs: Number of elements in @refs array -* @refs: Array of references with optional arguments -*/ -struct software_node_reference { -const char *name; -unsigned int nrefs; -const struct software_node_ref_args *refs; -}; -/** * struct software_node - Software node description * @name: Name of the software node * @parent: Parent of the software node * @properties: Array of device properties -* @references: Array of software node reference properties */ struct software_node {const char *name;const struct software_node *parent;const struct property_entry *properties; -const struct software_node_reference *references; };bool is_software_node (const struct fwnode_handle *fwnode);
Signature · 1 line
2.23.0.187.g17f5b7556c-goog
Quoted 36 lines · CEST Dmitry Torokhov
These series implement "references" properties for software nodes as true properties, instead of managing them completely separately.
The first 10 patches are generic cleanups and consolidation and unification of the existing code; patch #11 implements
PROPERTY_EMTRY_REF()and friends; patch #12 converts the user of references to the property syntax, and patch #13 removes the remains of references as entities that are managed separately.Changes in v3:
- added various cleanups before implementing reference propertiesChanges in v2:
- reworked code so that even single-entry reference properties are stored as arrays (i.e. thesoftware_node_ref_argsinstances are not part ofproperty_entrystructure) to avoid size increase. From user's POV nothing is changed, one can still usePROPERTY_ENTRY_REFmacro to define reference "inline".
- dropped unusedDEV_PROP_MAX
- rebased on linux-nextDmitry Torokhov (13):
software node: removeDEV_PROP_MAX
software node: clean upproperty_copy_string_array()software node: get rid ofproperty_set_pointer()software node: simplifyproperty_get_pointer()software node: removeproperty_entry_read_uNN_arrayfunctions software node: unifyPROPERTY_ENTRY_XXXmacros software node: simplifyproperty_entry_read_string_array()software node: introducePROPERTY_ENTRY_ARRAY_XXX_LEN()efi/apple-properties: usePROPERTY_ENTRY_U8_ARRAY_LENsoftware node: renameis_arraytois_inline
software node: implement reference properties platform/x86:intel_cht_int33fe: use inline reference properties software node: remove separate handling of references
Quoted 5 lines
drivers/base/swnode.c | 243 +++++++---------------- drivers/firmware/efi/apple-properties.c | 8 +- drivers/platform/x86/intel_cht_int33fe.c | 81 ++++---- include/linux/property.h | 154 +++++++------- 4 files changed, 198 insertions(+), 288 deletions(-)
I think that this is still relevant, so can you please resend it with a CC to [email protected]? It would be much easier to handle for me then.