* [PATCH v3 01/13] software node: remove DEV_PROP_MAX
2019-09-09 8:15 [PATCH v3 00/13] software node: add support for reference properties Dmitry Torokhov
@ 2019-09-09 8:15 ` Dmitry Torokhov
2019-09-09 9:46 ` Andy Shevchenko
2019-09-09 8:15 ` [PATCH v3 02/13] software node: clean up property_copy_string_array() Dmitry Torokhov
` (12 subsequent siblings)
13 siblings, 1 reply; 20+ messages in thread
From: Dmitry Torokhov @ 2019-09-09 8:15 UTC (permalink / raw)
To: Rafael J. Wysocki, Heikki Krogerus
Cc: Andy Shevchenko, Linus Walleij, linux-kernel, platform-driver-x86
This definition is not used anywhere, let's remove it.
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
include/linux/property.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/include/linux/property.h b/include/linux/property.h
index 9b3d4ca3a73a..44c1704f7163 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -22,7 +22,6 @@ enum dev_prop_type {
DEV_PROP_U32,
DEV_PROP_U64,
DEV_PROP_STRING,
- DEV_PROP_MAX,
};
enum dev_dma_attr {
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v3 01/13] software node: remove DEV_PROP_MAX
2019-09-09 8:15 ` [PATCH v3 01/13] software node: remove DEV_PROP_MAX Dmitry Torokhov
@ 2019-09-09 9:46 ` Andy Shevchenko
0 siblings, 0 replies; 20+ messages in thread
From: Andy Shevchenko @ 2019-09-09 9:46 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rafael J. Wysocki, Heikki Krogerus, Linus Walleij, linux-kernel,
platform-driver-x86
On Mon, Sep 09, 2019 at 01:15:45AM -0700, Dmitry Torokhov wrote:
> This definition is not used anywhere, let's remove it.
>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> include/linux/property.h | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/include/linux/property.h b/include/linux/property.h
> index 9b3d4ca3a73a..44c1704f7163 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -22,7 +22,6 @@ enum dev_prop_type {
> DEV_PROP_U32,
> DEV_PROP_U64,
> DEV_PROP_STRING,
> - DEV_PROP_MAX,
> };
>
> enum dev_dma_attr {
> --
> 2.23.0.187.g17f5b7556c-goog
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 02/13] software node: clean up property_copy_string_array()
2019-09-09 8:15 [PATCH v3 00/13] software node: add support for reference properties Dmitry Torokhov
2019-09-09 8:15 ` [PATCH v3 01/13] software node: remove DEV_PROP_MAX Dmitry Torokhov
@ 2019-09-09 8:15 ` Dmitry Torokhov
2019-09-09 8:15 ` [PATCH v3 03/13] software node: get rid of property_set_pointer() Dmitry Torokhov
` (11 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Dmitry Torokhov @ 2019-09-09 8:15 UTC (permalink / raw)
To: Rafael J. Wysocki, Heikki Krogerus
Cc: Andy Shevchenko, Linus Walleij, linux-kernel, platform-driver-x86
Because 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.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/base/swnode.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index ee2a405cca9a..7bad41a8f65d 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -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)
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 03/13] software node: get rid of property_set_pointer()
2019-09-09 8:15 [PATCH v3 00/13] software node: add support for reference properties Dmitry Torokhov
2019-09-09 8:15 ` [PATCH v3 01/13] software node: remove DEV_PROP_MAX Dmitry Torokhov
2019-09-09 8:15 ` [PATCH v3 02/13] software node: clean up property_copy_string_array() Dmitry Torokhov
@ 2019-09-09 8:15 ` Dmitry Torokhov
2019-09-09 9:55 ` Andy Shevchenko
2019-09-09 8:15 ` [PATCH v3 04/13] software node: simplify property_get_pointer() Dmitry Torokhov
` (10 subsequent siblings)
13 siblings, 1 reply; 20+ messages in thread
From: Dmitry Torokhov @ 2019-09-09 8:15 UTC (permalink / raw)
To: Rafael J. Wysocki, Heikki Krogerus
Cc: Andy Shevchenko, Linus Walleij, linux-kernel, platform-driver-x86
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().
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/base/swnode.c | 50 +++++-----------------------------------
include/linux/property.h | 1 +
2 files changed, 7 insertions(+), 44 deletions(-)
diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 7bad41a8f65d..a8d12046105e 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -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;
diff --git a/include/linux/property.h b/include/linux/property.h
index 44c1704f7163..4943b40d3536 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -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;
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v3 03/13] software node: get rid of property_set_pointer()
2019-09-09 8:15 ` [PATCH v3 03/13] software node: get rid of property_set_pointer() Dmitry Torokhov
@ 2019-09-09 9:55 ` Andy Shevchenko
2019-09-09 10:15 ` Dmitry Torokhov
0 siblings, 1 reply; 20+ messages in thread
From: Andy Shevchenko @ 2019-09-09 9:55 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rafael J. Wysocki, Heikki Krogerus, Linus Walleij, linux-kernel,
platform-driver-x86
On Mon, Sep 09, 2019 at 01:15:47AM -0700, Dmitry Torokhov wrote:
> 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.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 03/13] software node: get rid of property_set_pointer()
2019-09-09 9:55 ` Andy Shevchenko
@ 2019-09-09 10:15 ` Dmitry Torokhov
2019-09-09 11:36 ` Andy Shevchenko
0 siblings, 1 reply; 20+ messages in thread
From: Dmitry Torokhov @ 2019-09-09 10:15 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Rafael J. Wysocki, Heikki Krogerus, Linus Walleij, linux-kernel,
platform-driver-x86
On Mon, Sep 09, 2019 at 12:55:05PM +0300, Andy Shevchenko wrote:
> On Mon, Sep 09, 2019 at 01:15:47AM -0700, Dmitry Torokhov wrote:
> > 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?
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.
--
Dmitry
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 03/13] software node: get rid of property_set_pointer()
2019-09-09 10:15 ` Dmitry Torokhov
@ 2019-09-09 11:36 ` Andy Shevchenko
2019-09-09 13:48 ` Dmitry Torokhov
0 siblings, 1 reply; 20+ messages in thread
From: Andy Shevchenko @ 2019-09-09 11:36 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rafael J. Wysocki, Heikki Krogerus, Linus Walleij, linux-kernel,
platform-driver-x86
On Mon, Sep 09, 2019 at 03:15:55AM -0700, Dmitry Torokhov wrote:
> On Mon, Sep 09, 2019 at 12:55:05PM +0300, Andy Shevchenko wrote:
> > On Mon, Sep 09, 2019 at 01:15:47AM -0700, Dmitry Torokhov wrote:
> > > 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?
>
> 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.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 03/13] software node: get rid of property_set_pointer()
2019-09-09 11:36 ` Andy Shevchenko
@ 2019-09-09 13:48 ` Dmitry Torokhov
0 siblings, 0 replies; 20+ messages in thread
From: Dmitry Torokhov @ 2019-09-09 13:48 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Rafael J. Wysocki, Heikki Krogerus, Linus Walleij, linux-kernel,
platform-driver-x86
On Mon, Sep 09, 2019 at 02:36:52PM +0300, Andy Shevchenko wrote:
> On Mon, Sep 09, 2019 at 03:15:55AM -0700, Dmitry Torokhov wrote:
> > On Mon, Sep 09, 2019 at 12:55:05PM +0300, Andy Shevchenko wrote:
> > > On Mon, Sep 09, 2019 at 01:15:47AM -0700, Dmitry Torokhov wrote:
> > > > 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?
> >
> > 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 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.
>
> I dunno what others think about your proposal.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 04/13] software node: simplify property_get_pointer()
2019-09-09 8:15 [PATCH v3 00/13] software node: add support for reference properties Dmitry Torokhov
` (2 preceding siblings ...)
2019-09-09 8:15 ` [PATCH v3 03/13] software node: get rid of property_set_pointer() Dmitry Torokhov
@ 2019-09-09 8:15 ` Dmitry Torokhov
2019-09-09 8:15 ` [PATCH v3 05/13] software node: remove property_entry_read_uNN_array functions Dmitry Torokhov
` (9 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Dmitry Torokhov @ 2019-09-09 8:15 UTC (permalink / raw)
To: Rafael J. Wysocki, Heikki Krogerus
Cc: Andy Shevchenko, Linus Walleij, linux-kernel, platform-driver-x86
We do not need to handle each data type separately, we can simply return either
the raw pointer or pointer to values union.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/base/swnode.c | 29 ++++++-----------------------
1 file changed, 6 insertions(+), 23 deletions(-)
diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index a8d12046105e..bedc26189bed 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -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,
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 05/13] software node: remove property_entry_read_uNN_array functions
2019-09-09 8:15 [PATCH v3 00/13] software node: add support for reference properties Dmitry Torokhov
` (3 preceding siblings ...)
2019-09-09 8:15 ` [PATCH v3 04/13] software node: simplify property_get_pointer() Dmitry Torokhov
@ 2019-09-09 8:15 ` Dmitry Torokhov
2019-09-09 8:15 ` [PATCH v3 06/13] software node: unify PROPERTY_ENTRY_XXX macros Dmitry Torokhov
` (8 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Dmitry Torokhov @ 2019-09-09 8:15 UTC (permalink / raw)
To: Rafael J. Wysocki, Heikki Krogerus
Cc: Andy Shevchenko, Linus Walleij, linux-kernel, platform-driver-x86
There is absolutely no reason to have them as we can handle it all nicely in
property_entry_read_int_array().
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/base/swnode.c | 85 +++++++------------------------------------
1 file changed, 14 insertions(+), 71 deletions(-)
diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index bedc26189bed..a0629365972d 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -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,
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 06/13] software node: unify PROPERTY_ENTRY_XXX macros
2019-09-09 8:15 [PATCH v3 00/13] software node: add support for reference properties Dmitry Torokhov
` (4 preceding siblings ...)
2019-09-09 8:15 ` [PATCH v3 05/13] software node: remove property_entry_read_uNN_array functions Dmitry Torokhov
@ 2019-09-09 8:15 ` Dmitry Torokhov
2019-09-09 8:15 ` [PATCH v3 07/13] software node: simplify property_entry_read_string_array() Dmitry Torokhov
` (7 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Dmitry Torokhov @ 2019-09-09 8:15 UTC (permalink / raw)
To: Rafael J. Wysocki, Heikki Krogerus
Cc: Andy Shevchenko, Linus Walleij, linux-kernel, platform-driver-x86
We can unify string properties initializer macros with integer
initializers.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
include/linux/property.h | 70 +++++++++++++++++-----------------------
1 file changed, 30 insertions(+), 40 deletions(-)
diff --git a/include/linux/property.h b/include/linux/property.h
index 4943b40d3536..911ace267247 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -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) { \
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 07/13] software node: simplify property_entry_read_string_array()
2019-09-09 8:15 [PATCH v3 00/13] software node: add support for reference properties Dmitry Torokhov
` (5 preceding siblings ...)
2019-09-09 8:15 ` [PATCH v3 06/13] software node: unify PROPERTY_ENTRY_XXX macros Dmitry Torokhov
@ 2019-09-09 8:15 ` Dmitry Torokhov
2019-09-09 8:15 ` [PATCH v3 08/13] software node: introduce PROPERTY_ENTRY_ARRAY_XXX_LEN() Dmitry Torokhov
` (6 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Dmitry Torokhov @ 2019-09-09 8:15 UTC (permalink / raw)
To: Rafael J. Wysocki, Heikki Krogerus
Cc: Andy Shevchenko, Linus Walleij, linux-kernel, platform-driver-x86
There 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.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/base/swnode.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index a0629365972d..2dfeeab919d4 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -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);
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 08/13] software node: introduce PROPERTY_ENTRY_ARRAY_XXX_LEN()
2019-09-09 8:15 [PATCH v3 00/13] software node: add support for reference properties Dmitry Torokhov
` (6 preceding siblings ...)
2019-09-09 8:15 ` [PATCH v3 07/13] software node: simplify property_entry_read_string_array() Dmitry Torokhov
@ 2019-09-09 8:15 ` Dmitry Torokhov
2019-09-09 8:15 ` [PATCH v3 09/13] efi/apple-properties: use PROPERTY_ENTRY_U8_ARRAY_LEN Dmitry Torokhov
` (5 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Dmitry Torokhov @ 2019-09-09 8:15 UTC (permalink / raw)
To: Rafael J. Wysocki, Heikki Krogerus, Ard Biesheuvel
Cc: Andy Shevchenko, Linus Walleij, linux-kernel, platform-driver-x86
Sometimes 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.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
include/linux/property.h | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/include/linux/property.h b/include/linux/property.h
index 911ace267247..793d05cbc3b2 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -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_, \
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 09/13] efi/apple-properties: use PROPERTY_ENTRY_U8_ARRAY_LEN
2019-09-09 8:15 [PATCH v3 00/13] software node: add support for reference properties Dmitry Torokhov
` (7 preceding siblings ...)
2019-09-09 8:15 ` [PATCH v3 08/13] software node: introduce PROPERTY_ENTRY_ARRAY_XXX_LEN() Dmitry Torokhov
@ 2019-09-09 8:15 ` Dmitry Torokhov
2019-09-09 8:15 ` [PATCH v3 10/13] software node: rename is_array to is_inline Dmitry Torokhov
` (4 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Dmitry Torokhov @ 2019-09-09 8:15 UTC (permalink / raw)
To: Rafael J. Wysocki, Heikki Krogerus, Ard Biesheuvel
Cc: Andy Shevchenko, Linus Walleij, linux-kernel, platform-driver-x86
Let's switch to using PROPERTY_ENTRY_U8_ARRAY_LEN() to initialize
property entries instead of poking into the structure directly.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/firmware/efi/apple-properties.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/firmware/efi/apple-properties.c b/drivers/firmware/efi/apple-properties.c
index 0e206c9e0d7a..6a09174979b0 100644
--- a/drivers/firmware/efi/apple-properties.c
+++ b/drivers/firmware/efi/apple-properties.c
@@ -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);
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 10/13] software node: rename is_array to is_inline
2019-09-09 8:15 [PATCH v3 00/13] software node: add support for reference properties Dmitry Torokhov
` (8 preceding siblings ...)
2019-09-09 8:15 ` [PATCH v3 09/13] efi/apple-properties: use PROPERTY_ENTRY_U8_ARRAY_LEN Dmitry Torokhov
@ 2019-09-09 8:15 ` Dmitry Torokhov
2019-09-09 8:15 ` [PATCH v3 11/13] software node: implement reference properties Dmitry Torokhov
` (3 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Dmitry Torokhov @ 2019-09-09 8:15 UTC (permalink / raw)
To: Rafael J. Wysocki, Heikki Krogerus
Cc: Andy Shevchenko, Linus Walleij, linux-kernel, platform-driver-x86
We 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.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/base/swnode.c | 9 +++++----
include/linux/property.h | 7 ++++---
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 2dfeeab919d4..03643f55e5b5 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -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;
}
diff --git a/include/linux/property.h b/include/linux/property.h
index 793d05cbc3b2..5180e23348d2 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -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_, \
}
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 11/13] software node: implement reference properties
2019-09-09 8:15 [PATCH v3 00/13] software node: add support for reference properties Dmitry Torokhov
` (9 preceding siblings ...)
2019-09-09 8:15 ` [PATCH v3 10/13] software node: rename is_array to is_inline Dmitry Torokhov
@ 2019-09-09 8:15 ` Dmitry Torokhov
2019-09-09 8:15 ` [PATCH v3 12/13] platform/x86: intel_cht_int33fe: use inline " Dmitry Torokhov
` (2 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Dmitry Torokhov @ 2019-09-09 8:15 UTC (permalink / raw)
To: Rafael J. Wysocki, Heikki Krogerus
Cc: Andy Shevchenko, Linus Walleij, linux-kernel, platform-driver-x86
It 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),
{ }
};
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/base/swnode.c | 46 +++++++++++++++++++++++++++++++++-------
include/linux/property.h | 46 ++++++++++++++++++++++++++++------------
2 files changed, 70 insertions(+), 22 deletions(-)
diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 03643f55e5b5..4aaad0c7b1eb 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -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;
}
diff --git a/include/linux/property.h b/include/linux/property.h
index 5180e23348d2..c9234fc43917 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -22,6 +22,7 @@ enum dev_prop_type {
DEV_PROP_U32,
DEV_PROP_U64,
DEV_PROP_STRING,
+ DEV_PROP_REF,
};
enum dev_dma_attr {
@@ -218,6 +219,20 @@ static inline int fwnode_property_count_u64(const struct fwnode_handle *fwnode,
return fwnode_property_read_u64_array(fwnode, propname, NULL, 0);
}
+struct software_node;
+
+/**
+ * struct software_node_ref_args - Reference property with additional arguments
+ * @node: Reference to a software node
+ * @nargs: Number of elements in @args array
+ * @args: Integer arguments
+ */
+struct software_node_ref_args {
+ const struct software_node *node;
+ unsigned int nargs;
+ u64 args[NR_FWNODE_REFERENCE_ARGS];
+};
+
/**
* struct property_entry - "Built-in" device property representation.
* @name: Name of the property.
@@ -240,6 +255,7 @@ struct property_entry {
const u32 *u32_data;
const u64 *u64_data;
const char * const *str;
+ const struct software_node_ref_args *ref;
const void *raw;
} pointer;
union {
@@ -279,6 +295,8 @@ struct property_entry {
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_REF_ARRAY(_name_, _val_) \
+ PROPERTY_ENTRY_ARRAY(_name_, ref, REF, _val_)
#define PROPERTY_ENTRY_ARRAY_LEN(_name_, _elem_, _Type_, _val_, _len_) \
(struct property_entry) { \
@@ -298,6 +316,8 @@ 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_) \
+ PROPERTY_ENTRY_ARRAY_LEN(_name_, ref, REF, _val_, _len)
#define PROPERTY_ENTRY_ELEMENT(_name_, _elem_, _Type_, _val_) \
(struct property_entry) { \
@@ -324,6 +344,18 @@ struct property_entry {
.name = _name_, \
}
+#define PROPERTY_ENTRY_REF(_name_, _ref_, ...) \
+(struct property_entry) { \
+ .name = _name_, \
+ .length = sizeof(struct software_node_ref_args), \
+ .type = DEV_PROP_REF, \
+ .pointer.ref = &(const struct software_node_ref_args) { \
+ .node = _ref_, \
+ .nargs = ARRAY_SIZE(((u64[]){ 0, ##__VA_ARGS__ })) - 1, \
+ .args = { __VA_ARGS__ }, \
+ }, \
+}
+
struct property_entry *
property_entries_dup(const struct property_entry *properties);
@@ -387,20 +419,6 @@ int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
/* -------------------------------------------------------------------------- */
/* Software fwnode support - when HW description is incomplete or missing */
-struct software_node;
-
-/**
- * struct software_node_ref_args - Reference with additional arguments
- * @node: Reference to a software node
- * @nargs: Number of elements in @args array
- * @args: Integer arguments
- */
-struct software_node_ref_args {
- const struct software_node *node;
- unsigned int nargs;
- u64 args[NR_FWNODE_REFERENCE_ARGS];
-};
-
/**
* struct software_node_reference - Named software node reference property
* @name: Name of the property
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 12/13] platform/x86: intel_cht_int33fe: use inline reference properties
2019-09-09 8:15 [PATCH v3 00/13] software node: add support for reference properties Dmitry Torokhov
` (10 preceding siblings ...)
2019-09-09 8:15 ` [PATCH v3 11/13] software node: implement reference properties Dmitry Torokhov
@ 2019-09-09 8:15 ` Dmitry Torokhov
2019-09-09 8:15 ` [PATCH v3 13/13] software node: remove separate handling of references Dmitry Torokhov
2019-10-11 9:20 ` [PATCH v3 00/13] software node: add support for reference properties Rafael J. Wysocki
13 siblings, 0 replies; 20+ messages in thread
From: Dmitry Torokhov @ 2019-09-09 8:15 UTC (permalink / raw)
To: Rafael J. Wysocki, Heikki Krogerus
Cc: Andy Shevchenko, Linus Walleij, linux-kernel, platform-driver-x86
Now 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.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/platform/x86/intel_cht_int33fe.c | 81 ++++++++++++------------
1 file changed, 41 insertions(+), 40 deletions(-)
diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c
index 1d5d877b9582..4177c5424931 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -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
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 13/13] software node: remove separate handling of references
2019-09-09 8:15 [PATCH v3 00/13] software node: add support for reference properties Dmitry Torokhov
` (11 preceding siblings ...)
2019-09-09 8:15 ` [PATCH v3 12/13] platform/x86: intel_cht_int33fe: use inline " Dmitry Torokhov
@ 2019-09-09 8:15 ` Dmitry Torokhov
2019-10-11 9:20 ` [PATCH v3 00/13] software node: add support for reference properties Rafael J. Wysocki
13 siblings, 0 replies; 20+ messages in thread
From: Dmitry Torokhov @ 2019-09-09 8:15 UTC (permalink / raw)
To: Rafael J. Wysocki, Heikki Krogerus
Cc: Andy Shevchenko, Linus Walleij, linux-kernel, platform-driver-x86
Now that all users of references have moved to reference properties,
we can remove separate handling of references.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/base/swnode.c | 44 +++++++++++++++-------------------------
include/linux/property.h | 14 -------------
2 files changed, 16 insertions(+), 42 deletions(-)
diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 4aaad0c7b1eb..8085b39e23b4 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -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;
}
diff --git a/include/linux/property.h b/include/linux/property.h
index c9234fc43917..67cd38936c1e 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -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);
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v3 00/13] software node: add support for reference properties
2019-09-09 8:15 [PATCH v3 00/13] software node: add support for reference properties Dmitry Torokhov
` (12 preceding siblings ...)
2019-09-09 8:15 ` [PATCH v3 13/13] software node: remove separate handling of references Dmitry Torokhov
@ 2019-10-11 9:20 ` Rafael J. Wysocki
13 siblings, 0 replies; 20+ messages in thread
From: Rafael J. Wysocki @ 2019-10-11 9:20 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rafael J. Wysocki, Heikki Krogerus, Andy Shevchenko,
Linus Walleij, linux-kernel, platform-driver-x86
On Monday, September 9, 2019 10:15:44 AM CEST Dmitry Torokhov wrote:
> 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(-)
I think that this is still relevant, so can you please resend it with a CC
to linux-acpi@vger.kernel.org? It would be much easier to handle for me then.
^ permalink raw reply [flat|nested] 20+ messages in thread