* [PATCH v8 0/6] software node: add support for reference properties @ 2019-11-08 4:22 Dmitry Torokhov 2019-11-08 4:22 ` [PATCH v8 1/6] software node: rename is_array to is_inline Dmitry Torokhov ` (5 more replies) 0 siblings, 6 replies; 28+ messages in thread From: Dmitry Torokhov @ 2019-11-08 4:22 UTC (permalink / raw) To: Rafael J. Wysocki, Heikki Krogerus Cc: Andy Shevchenko, Mika Westerberg, Linus Walleij, Ard Biesheuvel, linux-acpi, linux-kernel, platform-driver-x86 These series implement "references" properties for software nodes as true properties, instead of managing them completely separately. The first 2 patches do away with separate handling of arrays vs single-value properties, and treat everything as arrays (which really matches how we handle OF or ACPI properties). Instead we recognize that data, if it is small enough, may be embedded into property_entry structure. As a side effect, properties can be converted from having their data stored separately to embedding their data when they are being copied. Patch #3 implements PROPERTY_ENTRY_REF() and friends; patch #4 converts the user of references to the property syntax, and patch #5 removes the remains of references as entities that are managed separately. Patch #6 adds unit tests to verify that the handling of property entries is correct. Note that patches #4 and $5 can be postponed. Changes in v8: - switch data members of "value" union to be arrays to clearly show that even when embedding we may be dealing with more than one element - when parsing references use property_entry_read_int_array() instead of fetching u32 value directly from nargs property Changes in v7: - rebased onto next-20191107 - dropped already applied patches - reworked patch that moved small properties inline on copying to avoid temporary allocation - cleaned up logic for embedding vs storing values out-of-line - fixed handling of embedded 2-element string array on x32 Changes in v6: - rebased onto next-20191023 - fixed patch moving small properties inline - fixed handling boolean properties after is_array -> is_inline conversion - changed comments around is_inline "stored directly" vs embedded in one place (Andy) - added unit tests for property entries based on KUnit framework - added Any's reviewed-by/acked-by Changes in v5: - rebased onto next-20191011 Changes in v4: - dealt with union aliasing concerns - inline small properties on copy 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 (6): software node: rename is_array to is_inline software node: allow embedding of small arrays into property_entry software node: implement reference properties platform/x86: intel_cht_int33fe: use inline reference properties software node: remove separate handling of references software node: add basic tests for property entries drivers/base/swnode.c | 154 +++--- drivers/base/test/Makefile | 2 + drivers/base/test/property-entry-test.c | 472 ++++++++++++++++++ .../platform/x86/intel_cht_int33fe_typec.c | 81 +-- include/linux/property.h | 98 ++-- 5 files changed, 655 insertions(+), 152 deletions(-) create mode 100644 drivers/base/test/property-entry-test.c -- 2.24.0.rc1.363.gb1bccd3e3d-goog ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v8 1/6] software node: rename is_array to is_inline 2019-11-08 4:22 [PATCH v8 0/6] software node: add support for reference properties Dmitry Torokhov @ 2019-11-08 4:22 ` Dmitry Torokhov 2019-11-08 9:49 ` Rafael J. Wysocki ` (2 more replies) 2019-11-08 4:22 ` [PATCH v8 2/6] software node: allow embedding of small arrays into property_entry Dmitry Torokhov ` (4 subsequent siblings) 5 siblings, 3 replies; 28+ messages in thread From: Dmitry Torokhov @ 2019-11-08 4:22 UTC (permalink / raw) To: Rafael J. Wysocki, Heikki Krogerus Cc: Andy Shevchenko, Mika Westerberg, Linus Walleij, Ard Biesheuvel, linux-acpi, 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 ratio 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 | 12 +++++------- include/linux/property.h | 13 ++++++++----- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c index d8d0dc0ca5acf..18a30fb3cc588 100644 --- a/drivers/base/swnode.c +++ b/drivers/base/swnode.c @@ -108,10 +108,7 @@ static const void *property_get_pointer(const struct property_entry *prop) if (!prop->length) return NULL; - if (prop->is_array) - return prop->pointer; - - return &prop->value; + return prop->is_inline ? &prop->value : prop->pointer; } static const void *property_entry_find(const struct property_entry *props, @@ -205,7 +202,7 @@ static void property_entry_free_data(const struct property_entry *p) const char * const *src_str; size_t i, nval; - if (p->is_array) { + if (!p->is_inline) { if (p->type == DEV_PROP_STRING && p->pointer) { src_str = p->pointer; nval = p->length / sizeof(const char *); @@ -250,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; @@ -264,15 +261,16 @@ static int property_entry_copy_data(struct property_entry *dst, return -ENOMEM; } - dst->is_array = true; dst->pointer = 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 48335288c2a96..dad0ad11b55e2 100644 --- a/include/linux/property.h +++ b/include/linux/property.h @@ -227,15 +227,17 @@ 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 embedded 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). + * @pointer: Pointer to the property when it is stored separately from + * the &struct property_entry instance. + * @value: Value of the property when it is stored inline. */ struct property_entry { const char *name; size_t length; - bool is_array; + bool is_inline; enum dev_prop_type type; union { const void *pointer; @@ -262,7 +264,6 @@ struct property_entry { (struct property_entry) { \ .name = _name_, \ .length = (_len_) * __PROPERTY_ENTRY_ELEMENT_SIZE(_elem_), \ - .is_array = true, \ .type = DEV_PROP_##_Type_, \ { .pointer = _val_ }, \ } @@ -293,6 +294,7 @@ struct property_entry { (struct property_entry) { \ .name = _name_, \ .length = __PROPERTY_ENTRY_ELEMENT_SIZE(_elem_), \ + .is_inline = true, \ .type = DEV_PROP_##_Type_, \ { .value = { ._elem_ = _val_ } }, \ } @@ -311,6 +313,7 @@ struct property_entry { #define PROPERTY_ENTRY_BOOL(_name_) \ (struct property_entry) { \ .name = _name_, \ + .is_inline = true, \ } struct property_entry * -- 2.24.0.rc1.363.gb1bccd3e3d-goog ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v8 1/6] software node: rename is_array to is_inline 2019-11-08 4:22 ` [PATCH v8 1/6] software node: rename is_array to is_inline Dmitry Torokhov @ 2019-11-08 9:49 ` Rafael J. Wysocki 2019-11-13 6:52 ` Bjørn Mork [not found] ` <CGME20191212111237eucas1p1a278d2d5d2437e3219896367e82604cc@eucas1p1.samsung.com> 2 siblings, 0 replies; 28+ messages in thread From: Rafael J. Wysocki @ 2019-11-08 9:49 UTC (permalink / raw) To: Dmitry Torokhov Cc: Rafael J. Wysocki, Heikki Krogerus, Andy Shevchenko, Mika Westerberg, Linus Walleij, Ard Biesheuvel, ACPI Devel Maling List, Linux Kernel Mailing List, Platform Driver On Fri, Nov 8, 2019 at 5:22 AM Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > > We do not need a special flag to know if we are dealing with an array, > as we can get that data from ratio 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. So the subject is slightly misleading, because it is not a rename. I would say "replace x with y" instead. [Arguably I can change that when applying the patch, but since we are going to wait for the dependencies to go in, it should not be a big deal to send an update of this patch alone?] > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> > --- > drivers/base/swnode.c | 12 +++++------- > include/linux/property.h | 13 ++++++++----- > 2 files changed, 13 insertions(+), 12 deletions(-) > > diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c > index d8d0dc0ca5acf..18a30fb3cc588 100644 > --- a/drivers/base/swnode.c > +++ b/drivers/base/swnode.c > @@ -108,10 +108,7 @@ static const void *property_get_pointer(const struct property_entry *prop) > if (!prop->length) > return NULL; > > - if (prop->is_array) > - return prop->pointer; > - > - return &prop->value; > + return prop->is_inline ? &prop->value : prop->pointer; > } > > static const void *property_entry_find(const struct property_entry *props, > @@ -205,7 +202,7 @@ static void property_entry_free_data(const struct property_entry *p) > const char * const *src_str; > size_t i, nval; > > - if (p->is_array) { > + if (!p->is_inline) { > if (p->type == DEV_PROP_STRING && p->pointer) { > src_str = p->pointer; > nval = p->length / sizeof(const char *); > @@ -250,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; > > @@ -264,15 +261,16 @@ static int property_entry_copy_data(struct property_entry *dst, > return -ENOMEM; > } > > - dst->is_array = true; > dst->pointer = 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 48335288c2a96..dad0ad11b55e2 100644 > --- a/include/linux/property.h > +++ b/include/linux/property.h > @@ -227,15 +227,17 @@ 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 embedded 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). > + * @pointer: Pointer to the property when it is stored separately from > + * the &struct property_entry instance. > + * @value: Value of the property when it is stored inline. And while at it, can you please try to make the comments shorter so they each take one line? ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v8 1/6] software node: rename is_array to is_inline 2019-11-08 4:22 ` [PATCH v8 1/6] software node: rename is_array to is_inline Dmitry Torokhov 2019-11-08 9:49 ` Rafael J. Wysocki @ 2019-11-13 6:52 ` Bjørn Mork 2019-11-13 8:08 ` Dmitry Torokhov [not found] ` <CGME20191212111237eucas1p1a278d2d5d2437e3219896367e82604cc@eucas1p1.samsung.com> 2 siblings, 1 reply; 28+ messages in thread From: Bjørn Mork @ 2019-11-13 6:52 UTC (permalink / raw) To: Dmitry Torokhov Cc: Rafael J. Wysocki, Heikki Krogerus, Andy Shevchenko, Mika Westerberg, Linus Walleij, Ard Biesheuvel, linux-acpi, linux-kernel, platform-driver-x86 Dmitry Torokhov <dmitry.torokhov@gmail.com> writes: > We do not need a special flag to know if we are dealing with an array, > as we can get that data from ratio 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. Doesn't a non-null prop->pointer tell you this? And inverting the flag is unnecessarily risky IMHO. An all-zero prop might now result in dereferencing a NULL prop->pointer instead of using the empty prop->value. Now I haven't looked at the code to see if this is a real problem. But I believe it's better not having to do that anyway... Bjørn ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v8 1/6] software node: rename is_array to is_inline 2019-11-13 6:52 ` Bjørn Mork @ 2019-11-13 8:08 ` Dmitry Torokhov 0 siblings, 0 replies; 28+ messages in thread From: Dmitry Torokhov @ 2019-11-13 8:08 UTC (permalink / raw) To: Bjørn Mork Cc: Rafael J. Wysocki, Heikki Krogerus, Andy Shevchenko, Mika Westerberg, Linus Walleij, Ard Biesheuvel, linux-acpi, linux-kernel, platform-driver-x86 On Wed, Nov 13, 2019 at 07:52:43AM +0100, Bjørn Mork wrote: > Dmitry Torokhov <dmitry.torokhov@gmail.com> writes: > > > We do not need a special flag to know if we are dealing with an array, > > as we can get that data from ratio 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. > > Doesn't a non-null prop->pointer tell you this? No it does not because pointer is a part of a union. > > And inverting the flag is unnecessarily risky IMHO. An all-zero prop > might now result in dereferencing a NULL prop->pointer instead of using > the empty prop->value. Now I haven't looked at the code to see if this > is a real problem. But I believe it's better not having to do that > anyway... All-zero property is a terminator and thus we will not dereference anything. Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 28+ messages in thread
[parent not found: <CGME20191212111237eucas1p1a278d2d5d2437e3219896367e82604cc@eucas1p1.samsung.com>]
* Re: [PATCH v8 1/6] software node: rename is_array to is_inline [not found] ` <CGME20191212111237eucas1p1a278d2d5d2437e3219896367e82604cc@eucas1p1.samsung.com> @ 2019-12-12 11:12 ` Marek Szyprowski 2019-12-12 11:28 ` Andy Shevchenko 2019-12-13 1:24 ` Dmitry Torokhov 0 siblings, 2 replies; 28+ messages in thread From: Marek Szyprowski @ 2019-12-12 11:12 UTC (permalink / raw) To: Dmitry Torokhov, Rafael J. Wysocki, Heikki Krogerus Cc: Andy Shevchenko, Mika Westerberg, Linus Walleij, Ard Biesheuvel, linux-acpi, linux-kernel, platform-driver-x86, 'Linux Samsung SOC', Krzysztof Kozlowski, Felipe Balbi, Linux USB Mailing List Dear All, On 08.11.2019 05:22, Dmitry Torokhov wrote: > We do not need a special flag to know if we are dealing with an array, > as we can get that data from ratio 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> Today I've noticed that this patch got merged to linux-next as commit e6bff4665c595b5a4aff173848851ed49ac3bfad. Sadly it breaks DWC3/xHCI driver operation on Samsung Exynos5 SoCs (and probably on other SoCs which use DWC3 in host mode too). I get the following errors during boot: dwc3 12000000.dwc3: failed to add properties to xHCI dwc3 12000000.dwc3: failed to initialize host dwc3: probe of 12000000.dwc3 failed with error -61 Here is a full kernel log from Exynos5250-based Snow Chromebook on KernelCI: https://storage.kernelci.org/next/master/next-20191212/arm/exynos_defconfig/gcc-8/lab-collabora/boot-exynos5250-snow.txt (lack of 'ref' clk is not related nor fatal to the driver operation). The code which fails after this patch is located in drivers/usb/dwc3/host.c. Let me know if I can help more in locating the bug. > --- > drivers/base/swnode.c | 12 +++++------- > include/linux/property.h | 13 ++++++++----- > 2 files changed, 13 insertions(+), 12 deletions(-) > > diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c > index d8d0dc0ca5acf..18a30fb3cc588 100644 > --- a/drivers/base/swnode.c > +++ b/drivers/base/swnode.c > @@ -108,10 +108,7 @@ static const void *property_get_pointer(const struct property_entry *prop) > if (!prop->length) > return NULL; > > - if (prop->is_array) > - return prop->pointer; > - > - return &prop->value; > + return prop->is_inline ? &prop->value : prop->pointer; > } > > static const void *property_entry_find(const struct property_entry *props, > @@ -205,7 +202,7 @@ static void property_entry_free_data(const struct property_entry *p) > const char * const *src_str; > size_t i, nval; > > - if (p->is_array) { > + if (!p->is_inline) { > if (p->type == DEV_PROP_STRING && p->pointer) { > src_str = p->pointer; > nval = p->length / sizeof(const char *); > @@ -250,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; > > @@ -264,15 +261,16 @@ static int property_entry_copy_data(struct property_entry *dst, > return -ENOMEM; > } > > - dst->is_array = true; > dst->pointer = 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 48335288c2a96..dad0ad11b55e2 100644 > --- a/include/linux/property.h > +++ b/include/linux/property.h > @@ -227,15 +227,17 @@ 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 embedded 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). > + * @pointer: Pointer to the property when it is stored separately from > + * the &struct property_entry instance. > + * @value: Value of the property when it is stored inline. > */ > struct property_entry { > const char *name; > size_t length; > - bool is_array; > + bool is_inline; > enum dev_prop_type type; > union { > const void *pointer; > @@ -262,7 +264,6 @@ struct property_entry { > (struct property_entry) { \ > .name = _name_, \ > .length = (_len_) * __PROPERTY_ENTRY_ELEMENT_SIZE(_elem_), \ > - .is_array = true, \ > .type = DEV_PROP_##_Type_, \ > { .pointer = _val_ }, \ > } > @@ -293,6 +294,7 @@ struct property_entry { > (struct property_entry) { \ > .name = _name_, \ > .length = __PROPERTY_ENTRY_ELEMENT_SIZE(_elem_), \ > + .is_inline = true, \ > .type = DEV_PROP_##_Type_, \ > { .value = { ._elem_ = _val_ } }, \ > } > @@ -311,6 +313,7 @@ struct property_entry { > #define PROPERTY_ENTRY_BOOL(_name_) \ > (struct property_entry) { \ > .name = _name_, \ > + .is_inline = true, \ > } > > struct property_entry * Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v8 1/6] software node: rename is_array to is_inline 2019-12-12 11:12 ` Marek Szyprowski @ 2019-12-12 11:28 ` Andy Shevchenko 2019-12-12 16:41 ` Rafael J. Wysocki 2019-12-13 1:24 ` Dmitry Torokhov 1 sibling, 1 reply; 28+ messages in thread From: Andy Shevchenko @ 2019-12-12 11:28 UTC (permalink / raw) To: Marek Szyprowski Cc: Dmitry Torokhov, Rafael J. Wysocki, Heikki Krogerus, Mika Westerberg, Linus Walleij, Ard Biesheuvel, linux-acpi, linux-kernel, platform-driver-x86, 'Linux Samsung SOC', Krzysztof Kozlowski, Felipe Balbi, Linux USB Mailing List On Thu, Dec 12, 2019 at 12:12:36PM +0100, Marek Szyprowski wrote: > Dear All, > > On 08.11.2019 05:22, Dmitry Torokhov wrote: > > We do not need a special flag to know if we are dealing with an array, > > as we can get that data from ratio 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> > > Today I've noticed that this patch got merged to linux-next as commit > e6bff4665c595b5a4aff173848851ed49ac3bfad. Sadly it breaks DWC3/xHCI > driver operation on Samsung Exynos5 SoCs (and probably on other SoCs > which use DWC3 in host mode too). I get the following errors during boot: > > dwc3 12000000.dwc3: failed to add properties to xHCI > dwc3 12000000.dwc3: failed to initialize host > dwc3: probe of 12000000.dwc3 failed with error -61 > > Here is a full kernel log from Exynos5250-based Snow Chromebook on KernelCI: > > https://storage.kernelci.org/next/master/next-20191212/arm/exynos_defconfig/gcc-8/lab-collabora/boot-exynos5250-snow.txt > > (lack of 'ref' clk is not related nor fatal to the driver operation). > > The code which fails after this patch is located in > drivers/usb/dwc3/host.c. Let me know if I can help more in locating the bug. Thank you for report. I think we should not have that patch in the fist place... I used to have a bad feeling about it and then forgot about it existence. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v8 1/6] software node: rename is_array to is_inline 2019-12-12 11:28 ` Andy Shevchenko @ 2019-12-12 16:41 ` Rafael J. Wysocki 2019-12-13 6:47 ` Marek Szyprowski 0 siblings, 1 reply; 28+ messages in thread From: Rafael J. Wysocki @ 2019-12-12 16:41 UTC (permalink / raw) To: Andy Shevchenko Cc: Marek Szyprowski, Dmitry Torokhov, Rafael J. Wysocki, Heikki Krogerus, Mika Westerberg, Linus Walleij, Ard Biesheuvel, ACPI Devel Maling List, Linux Kernel Mailing List, Platform Driver, Linux Samsung SOC, Krzysztof Kozlowski, Felipe Balbi, Linux USB Mailing List On Thu, Dec 12, 2019 at 12:28 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Dec 12, 2019 at 12:12:36PM +0100, Marek Szyprowski wrote: > > Dear All, > > > > On 08.11.2019 05:22, Dmitry Torokhov wrote: > > > We do not need a special flag to know if we are dealing with an array, > > > as we can get that data from ratio 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> > > > > Today I've noticed that this patch got merged to linux-next as commit > > e6bff4665c595b5a4aff173848851ed49ac3bfad. Sadly it breaks DWC3/xHCI > > driver operation on Samsung Exynos5 SoCs (and probably on other SoCs > > which use DWC3 in host mode too). I get the following errors during boot: > > > > dwc3 12000000.dwc3: failed to add properties to xHCI > > dwc3 12000000.dwc3: failed to initialize host > > dwc3: probe of 12000000.dwc3 failed with error -61 > > > > Here is a full kernel log from Exynos5250-based Snow Chromebook on KernelCI: > > > > https://storage.kernelci.org/next/master/next-20191212/arm/exynos_defconfig/gcc-8/lab-collabora/boot-exynos5250-snow.txt > > > > (lack of 'ref' clk is not related nor fatal to the driver operation). > > > > The code which fails after this patch is located in > > drivers/usb/dwc3/host.c. Let me know if I can help more in locating the bug. > > Thank you for report. > > I think we should not have that patch in the fist place... I used to have > a bad feeling about it and then forgot about it existence. Well, I think you mean the [2/6]. The $subject one really shouldn't change functionality, we must have missed something here. Anyway, I'll drop this branch from the linux-next one for now. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v8 1/6] software node: rename is_array to is_inline 2019-12-12 16:41 ` Rafael J. Wysocki @ 2019-12-13 6:47 ` Marek Szyprowski 2019-12-13 8:37 ` Rafael J. Wysocki 0 siblings, 1 reply; 28+ messages in thread From: Marek Szyprowski @ 2019-12-13 6:47 UTC (permalink / raw) To: Rafael J. Wysocki, Andy Shevchenko Cc: Dmitry Torokhov, Heikki Krogerus, Mika Westerberg, Linus Walleij, Ard Biesheuvel, ACPI Devel Maling List, Linux Kernel Mailing List, Platform Driver, Linux Samsung SOC, Krzysztof Kozlowski, Felipe Balbi, Linux USB Mailing List Hi Rafael, On 12.12.2019 17:41, Rafael J. Wysocki wrote: > On Thu, Dec 12, 2019 at 12:28 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: >> On Thu, Dec 12, 2019 at 12:12:36PM +0100, Marek Szyprowski wrote: >>> On 08.11.2019 05:22, Dmitry Torokhov wrote: >>>> We do not need a special flag to know if we are dealing with an array, >>>> as we can get that data from ratio 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> >>> Today I've noticed that this patch got merged to linux-next as commit >>> e6bff4665c595b5a4aff173848851ed49ac3bfad. Sadly it breaks DWC3/xHCI >>> driver operation on Samsung Exynos5 SoCs (and probably on other SoCs >>> which use DWC3 in host mode too). I get the following errors during boot: >>> >>> dwc3 12000000.dwc3: failed to add properties to xHCI >>> dwc3 12000000.dwc3: failed to initialize host >>> dwc3: probe of 12000000.dwc3 failed with error -61 >>> >>> Here is a full kernel log from Exynos5250-based Snow Chromebook on KernelCI: >>> >>> https://storage.kernelci.org/next/master/next-20191212/arm/exynos_defconfig/gcc-8/lab-collabora/boot-exynos5250-snow.txt >>> >>> (lack of 'ref' clk is not related nor fatal to the driver operation). >>> >>> The code which fails after this patch is located in >>> drivers/usb/dwc3/host.c. Let me know if I can help more in locating the bug. >> Thank you for report. >> >> I think we should not have that patch in the fist place... I used to have >> a bad feeling about it and then forgot about it existence. > Well, I think you mean the [2/6]. > > The $subject one really shouldn't change functionality, we must have > missed something here. Nope, I was really talking about [1/6]. It looks that it revealed an issue in the DWC3 driver pointed by Dmitry. > Anyway, I'll drop this branch from the linux-next one for now. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v8 1/6] software node: rename is_array to is_inline 2019-12-13 6:47 ` Marek Szyprowski @ 2019-12-13 8:37 ` Rafael J. Wysocki 0 siblings, 0 replies; 28+ messages in thread From: Rafael J. Wysocki @ 2019-12-13 8:37 UTC (permalink / raw) To: Marek Szyprowski Cc: Rafael J. Wysocki, Andy Shevchenko, Dmitry Torokhov, Heikki Krogerus, Mika Westerberg, Linus Walleij, Ard Biesheuvel, ACPI Devel Maling List, Linux Kernel Mailing List, Platform Driver, Linux Samsung SOC, Krzysztof Kozlowski, Felipe Balbi, Linux USB Mailing List On Fri, Dec 13, 2019 at 7:47 AM Marek Szyprowski <m.szyprowski@samsung.com> wrote: > > Hi Rafael, > > On 12.12.2019 17:41, Rafael J. Wysocki wrote: > > On Thu, Dec 12, 2019 at 12:28 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > >> On Thu, Dec 12, 2019 at 12:12:36PM +0100, Marek Szyprowski wrote: > >>> On 08.11.2019 05:22, Dmitry Torokhov wrote: > >>>> We do not need a special flag to know if we are dealing with an array, > >>>> as we can get that data from ratio 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> > >>> Today I've noticed that this patch got merged to linux-next as commit > >>> e6bff4665c595b5a4aff173848851ed49ac3bfad. Sadly it breaks DWC3/xHCI > >>> driver operation on Samsung Exynos5 SoCs (and probably on other SoCs > >>> which use DWC3 in host mode too). I get the following errors during boot: > >>> > >>> dwc3 12000000.dwc3: failed to add properties to xHCI > >>> dwc3 12000000.dwc3: failed to initialize host > >>> dwc3: probe of 12000000.dwc3 failed with error -61 > >>> > >>> Here is a full kernel log from Exynos5250-based Snow Chromebook on KernelCI: > >>> > >>> https://storage.kernelci.org/next/master/next-20191212/arm/exynos_defconfig/gcc-8/lab-collabora/boot-exynos5250-snow.txt > >>> > >>> (lack of 'ref' clk is not related nor fatal to the driver operation). > >>> > >>> The code which fails after this patch is located in > >>> drivers/usb/dwc3/host.c. Let me know if I can help more in locating the bug. > >> Thank you for report. > >> > >> I think we should not have that patch in the fist place... I used to have > >> a bad feeling about it and then forgot about it existence. > > Well, I think you mean the [2/6]. > > > > The $subject one really shouldn't change functionality, we must have > > missed something here. > > Nope, I was really talking about [1/6]. It looks that it revealed an > issue in the DWC3 driver pointed by Dmitry. Right, but I was referring to the Andy's comment. Cheers! ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v8 1/6] software node: rename is_array to is_inline 2019-12-12 11:12 ` Marek Szyprowski 2019-12-12 11:28 ` Andy Shevchenko @ 2019-12-13 1:24 ` Dmitry Torokhov 2019-12-13 6:44 ` Marek Szyprowski 1 sibling, 1 reply; 28+ messages in thread From: Dmitry Torokhov @ 2019-12-13 1:24 UTC (permalink / raw) To: Marek Szyprowski Cc: Rafael J. Wysocki, Heikki Krogerus, Andy Shevchenko, Mika Westerberg, Linus Walleij, Ard Biesheuvel, linux-acpi, linux-kernel, platform-driver-x86, 'Linux Samsung SOC', Krzysztof Kozlowski, Felipe Balbi, Linux USB Mailing List Hi Marek, On Thu, Dec 12, 2019 at 12:12:36PM +0100, Marek Szyprowski wrote: > Dear All, > > On 08.11.2019 05:22, Dmitry Torokhov wrote: > > We do not need a special flag to know if we are dealing with an array, > > as we can get that data from ratio 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> > > Today I've noticed that this patch got merged to linux-next as commit > e6bff4665c595b5a4aff173848851ed49ac3bfad. Sadly it breaks DWC3/xHCI > driver operation on Samsung Exynos5 SoCs (and probably on other SoCs > which use DWC3 in host mode too). I get the following errors during boot: > > dwc3 12000000.dwc3: failed to add properties to xHCI > dwc3 12000000.dwc3: failed to initialize host > dwc3: probe of 12000000.dwc3 failed with error -61 > > Here is a full kernel log from Exynos5250-based Snow Chromebook on KernelCI: > > https://storage.kernelci.org/next/master/next-20191212/arm/exynos_defconfig/gcc-8/lab-collabora/boot-exynos5250-snow.txt > > (lack of 'ref' clk is not related nor fatal to the driver operation). > > The code which fails after this patch is located in > drivers/usb/dwc3/host.c. Let me know if I can help more in locating the bug. Does the following help? If, as I expect, it does, I'll submit it formally. --- diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c index 5567ed2cddbec..fa252870c926f 100644 --- a/drivers/usb/dwc3/host.c +++ b/drivers/usb/dwc3/host.c @@ -88,10 +88,10 @@ int dwc3_host_init(struct dwc3 *dwc) memset(props, 0, sizeof(struct property_entry) * ARRAY_SIZE(props)); if (dwc->usb3_lpm_capable) - props[prop_idx++].name = "usb3-lpm-capable"; + props[prop_idx++] = PROPERTY_ENTRY_BOOL("usb3-lpm-capable"); if (dwc->usb2_lpm_disable) - props[prop_idx++].name = "usb2-lpm-disable"; + props[prop_idx++] = PROPERTY_ENTRY_BOOL("usb2-lpm-disable"); /** * WORKAROUND: dwc3 revisions <=3.00a have a limitation @@ -103,7 +103,7 @@ int dwc3_host_init(struct dwc3 *dwc) * This following flag tells XHCI to do just that. */ if (dwc->revision <= DWC3_REVISION_300A) - props[prop_idx++].name = "quirk-broken-port-ped"; + props[prop_idx++] = PROPERTY_ENTRY_BOOL("quirk-broken-port-ped"); if (prop_idx) { ret = platform_device_add_properties(xhci, props); -- Dmitry ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v8 1/6] software node: rename is_array to is_inline 2019-12-13 1:24 ` Dmitry Torokhov @ 2019-12-13 6:44 ` Marek Szyprowski 0 siblings, 0 replies; 28+ messages in thread From: Marek Szyprowski @ 2019-12-13 6:44 UTC (permalink / raw) To: Dmitry Torokhov Cc: Rafael J. Wysocki, Heikki Krogerus, Andy Shevchenko, Mika Westerberg, Linus Walleij, Ard Biesheuvel, linux-acpi, linux-kernel, platform-driver-x86, 'Linux Samsung SOC', Krzysztof Kozlowski, Felipe Balbi, Linux USB Mailing List Hi Dmitry, On 13.12.2019 02:24, Dmitry Torokhov wrote: > On Thu, Dec 12, 2019 at 12:12:36PM +0100, Marek Szyprowski wrote: >> On 08.11.2019 05:22, Dmitry Torokhov wrote: >>> We do not need a special flag to know if we are dealing with an array, >>> as we can get that data from ratio 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> >> Today I've noticed that this patch got merged to linux-next as commit >> e6bff4665c595b5a4aff173848851ed49ac3bfad. Sadly it breaks DWC3/xHCI >> driver operation on Samsung Exynos5 SoCs (and probably on other SoCs >> which use DWC3 in host mode too). I get the following errors during boot: >> >> dwc3 12000000.dwc3: failed to add properties to xHCI >> dwc3 12000000.dwc3: failed to initialize host >> dwc3: probe of 12000000.dwc3 failed with error -61 >> >> Here is a full kernel log from Exynos5250-based Snow Chromebook on KernelCI: >> >> https://protect2.fireeye.com/url?k=df93ba76-820d14ec-df923139-0cc47a336fae-f751d8b108bc5bdf&u=https://storage.kernelci.org/next/master/next-20191212/arm/exynos_defconfig/gcc-8/lab-collabora/boot-exynos5250-snow.txt >> >> (lack of 'ref' clk is not related nor fatal to the driver operation). >> >> The code which fails after this patch is located in >> drivers/usb/dwc3/host.c. Let me know if I can help more in locating the bug. > Does the following help? If, as I expect, it does, I'll submit it > formally. Yes, it fixes the issue. If possible, please add the following tags: Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> to the final patch. > --- > > diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c > index 5567ed2cddbec..fa252870c926f 100644 > --- a/drivers/usb/dwc3/host.c > +++ b/drivers/usb/dwc3/host.c > @@ -88,10 +88,10 @@ int dwc3_host_init(struct dwc3 *dwc) > memset(props, 0, sizeof(struct property_entry) * ARRAY_SIZE(props)); > > if (dwc->usb3_lpm_capable) > - props[prop_idx++].name = "usb3-lpm-capable"; > + props[prop_idx++] = PROPERTY_ENTRY_BOOL("usb3-lpm-capable"); > > if (dwc->usb2_lpm_disable) > - props[prop_idx++].name = "usb2-lpm-disable"; > + props[prop_idx++] = PROPERTY_ENTRY_BOOL("usb2-lpm-disable"); > > /** > * WORKAROUND: dwc3 revisions <=3.00a have a limitation > @@ -103,7 +103,7 @@ int dwc3_host_init(struct dwc3 *dwc) > * This following flag tells XHCI to do just that. > */ > if (dwc->revision <= DWC3_REVISION_300A) > - props[prop_idx++].name = "quirk-broken-port-ped"; > + props[prop_idx++] = PROPERTY_ENTRY_BOOL("quirk-broken-port-ped"); > > if (prop_idx) { > ret = platform_device_add_properties(xhci, props); > > Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v8 2/6] software node: allow embedding of small arrays into property_entry 2019-11-08 4:22 [PATCH v8 0/6] software node: add support for reference properties Dmitry Torokhov 2019-11-08 4:22 ` [PATCH v8 1/6] software node: rename is_array to is_inline Dmitry Torokhov @ 2019-11-08 4:22 ` Dmitry Torokhov 2019-11-08 4:22 ` [PATCH v8 3/6] software node: implement reference properties Dmitry Torokhov ` (3 subsequent siblings) 5 siblings, 0 replies; 28+ messages in thread From: Dmitry Torokhov @ 2019-11-08 4:22 UTC (permalink / raw) To: Rafael J. Wysocki, Heikki Krogerus Cc: Andy Shevchenko, Mika Westerberg, Linus Walleij, Ard Biesheuvel, linux-acpi, linux-kernel, platform-driver-x86 We should not conflate whether a property data is an array or a single value with where it is stored (embedded into property_entry structure or out-of-line). All single-value properties are in effect 1-element arrays, and we can figure the amount of data stored in a property by examining its length and the data type. And arrays can be as easily stored in property entry instances as single values are, provided that we have enough space (we have up to 8 bytes). We can embed: - up to 8 bytes from U8 arrays - up to 4 words - up to 2 double words - one U64 value - one (on 64 bit architectures) or 2 (on 32 bit) strings. This change also has an effect of switching properties with small amount of data to embed it instead of keeping it separate when copying such properties. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- drivers/base/swnode.c | 115 +++++++++++++++++++-------------------- include/linux/property.h | 14 ++--- 2 files changed, 62 insertions(+), 67 deletions(-) diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c index 18a30fb3cc588..3d422918a53d9 100644 --- a/drivers/base/swnode.c +++ b/drivers/base/swnode.c @@ -198,93 +198,84 @@ static int property_entry_read_string_array(const struct property_entry *props, static void property_entry_free_data(const struct property_entry *p) { - const void *pointer = property_get_pointer(p); const char * const *src_str; size_t i, nval; - if (!p->is_inline) { - if (p->type == DEV_PROP_STRING && p->pointer) { - src_str = p->pointer; - nval = p->length / sizeof(const char *); - for (i = 0; i < nval; i++) - kfree(src_str[i]); - } - kfree(pointer); - } else if (p->type == DEV_PROP_STRING) { - kfree(p->value.str); + if (p->type == DEV_PROP_STRING) { + src_str = property_get_pointer(p); + nval = p->length / sizeof(*src_str); + for (i = 0; i < nval; i++) + kfree(src_str[i]); } + + if (!p->is_inline) + kfree(p->pointer); + kfree(p->name); } -static const char * const * -property_copy_string_array(const struct property_entry *src) +static bool property_copy_string_array(const char **dst_ptr, + const char * const *src_ptr, + size_t nval) { - const char **d; - const char * const *src_str = src->pointer; - size_t nval = src->length / sizeof(*d); int i; - d = kcalloc(nval, sizeof(*d), GFP_KERNEL); - if (!d) - return NULL; - for (i = 0; i < nval; i++) { - d[i] = kstrdup(src_str[i], GFP_KERNEL); - if (!d[i] && src_str[i]) { + dst_ptr[i] = kstrdup(src_ptr[i], GFP_KERNEL); + if (!dst_ptr[i] && src_ptr[i]) { while (--i >= 0) - kfree(d[i]); - kfree(d); - return NULL; + kfree(dst_ptr[i]); + return false; } } - return d; + return true; } static int property_entry_copy_data(struct property_entry *dst, const struct property_entry *src) { const void *pointer = property_get_pointer(src); - const void *new; - - if (!src->is_inline) { - if (!src->length) - return -ENODATA; - - if (src->type == DEV_PROP_STRING) { - new = property_copy_string_array(src); - if (!new) - return -ENOMEM; - } else { - new = kmemdup(pointer, src->length, GFP_KERNEL); - if (!new) - return -ENOMEM; - } - - dst->pointer = new; - } else if (src->type == DEV_PROP_STRING) { - new = kstrdup(src->value.str, GFP_KERNEL); - if (!new && src->value.str) + void *dst_ptr; + size_t nval; + + /* + * Properties with no data should not be marked as stored + * out of line. + */ + if (!src->is_inline && !src->length) + return -ENODATA; + + if (src->length <= sizeof(dst->value)) { + dst_ptr = &dst->value; + dst->is_inline = true; + } else { + dst_ptr = kmalloc(src->length, GFP_KERNEL); + if (!dst_ptr) return -ENOMEM; + dst->pointer = dst_ptr; + } - dst->is_inline = true; - dst->value.str = new; + if (src->type == DEV_PROP_STRING) { + nval = src->length / sizeof(const char *); + if (!property_copy_string_array(dst_ptr, pointer, nval)) { + if (!dst->is_inline) + kfree(dst->pointer); + return -ENOMEM; + } } else { - dst->is_inline = true; - dst->value = src->value; + memcpy(dst_ptr, pointer, src->length); } dst->length = src->length; dst->type = src->type; dst->name = kstrdup(src->name, GFP_KERNEL); - if (!dst->name) - goto out_free_data; + if (!dst->name) { + property_entry_free_data(dst); + return -ENOMEM; + } return 0; - -out_free_data: - property_entry_free_data(dst); - return -ENOMEM; } /** @@ -484,6 +475,8 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode, const struct software_node_reference *ref; const struct property_entry *prop; struct fwnode_handle *refnode; + u32 nargs_prop_val; + int error; int i; if (!swnode || !swnode->node->references) @@ -501,11 +494,13 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode, return -ENOENT; if (nargs_prop) { - prop = property_entry_get(swnode->node->properties, nargs_prop); - if (!prop) - return -EINVAL; + error = property_entry_read_int_array(swnode->node->properties, + nargs_prop, sizeof(u32), + &nargs_prop_val, 1); + if (error) + return error; - nargs = prop->value.u32_data; + nargs = nargs_prop_val; } if (nargs > NR_FWNODE_REFERENCE_ARGS) diff --git a/include/linux/property.h b/include/linux/property.h index dad0ad11b55e2..c592c286e3394 100644 --- a/include/linux/property.h +++ b/include/linux/property.h @@ -242,11 +242,11 @@ struct property_entry { union { const void *pointer; union { - u8 u8_data; - u16 u16_data; - u32 u32_data; - u64 u64_data; - const char *str; + u8 u8_data[sizeof(u64) / sizeof(u8)]; + u16 u16_data[sizeof(u64) / sizeof(u16)]; + u32 u32_data[sizeof(u64) / sizeof(u32)]; + u64 u64_data[sizeof(u64) / sizeof(u64)]; + const char *str[sizeof(u64) / sizeof(char *)]; } value; }; }; @@ -258,7 +258,7 @@ struct property_entry { */ #define __PROPERTY_ENTRY_ELEMENT_SIZE(_elem_) \ - sizeof(((struct property_entry *)NULL)->value._elem_) + sizeof(((struct property_entry *)NULL)->value._elem_[0]) #define __PROPERTY_ENTRY_ARRAY_LEN(_name_, _elem_, _Type_, _val_, _len_)\ (struct property_entry) { \ @@ -296,7 +296,7 @@ struct property_entry { .length = __PROPERTY_ENTRY_ELEMENT_SIZE(_elem_), \ .is_inline = true, \ .type = DEV_PROP_##_Type_, \ - { .value = { ._elem_ = _val_ } }, \ + { .value = { ._elem_[0] = _val_ } }, \ } #define PROPERTY_ENTRY_U8(_name_, _val_) \ -- 2.24.0.rc1.363.gb1bccd3e3d-goog ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v8 3/6] software node: implement reference properties 2019-11-08 4:22 [PATCH v8 0/6] software node: add support for reference properties Dmitry Torokhov 2019-11-08 4:22 ` [PATCH v8 1/6] software node: rename is_array to is_inline Dmitry Torokhov 2019-11-08 4:22 ` [PATCH v8 2/6] software node: allow embedding of small arrays into property_entry Dmitry Torokhov @ 2019-11-08 4:22 ` Dmitry Torokhov [not found] ` <CGME20201109170241eucas1p14c2156334d8c6ef15d52664fa4776f41@eucas1p1.samsung.com> 2019-11-08 4:22 ` [PATCH v8 4/6] platform/x86: intel_cht_int33fe: use inline " Dmitry Torokhov ` (2 subsequent siblings) 5 siblings, 1 reply; 28+ messages in thread From: Dmitry Torokhov @ 2019-11-08 4:22 UTC (permalink / raw) To: Rafael J. Wysocki, Heikki Krogerus Cc: Andy Shevchenko, Mika Westerberg, Linus Walleij, Ard Biesheuvel, linux-acpi, 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 | 49 ++++++++++++++++++++++++++++------ include/linux/property.h | 57 +++++++++++++++++++++++++++++----------- 2 files changed, 82 insertions(+), 24 deletions(-) diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c index 3d422918a53d9..604d7327bba79 100644 --- a/drivers/base/swnode.c +++ b/drivers/base/swnode.c @@ -246,6 +246,13 @@ static int property_entry_copy_data(struct property_entry *dst, if (!src->is_inline && !src->length) return -ENODATA; + /* + * Reference properties are never stored inline as + * they are too big. + */ + if (src->type == DEV_PROP_REF && src->is_inline) + return -EINVAL; + if (src->length <= sizeof(dst->value)) { dst_ptr = &dst->value; dst->is_inline = true; @@ -473,23 +480,49 @@ 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_array; + const struct software_node_ref_args *ref_args; const struct property_entry *prop; struct fwnode_handle *refnode; u32 nargs_prop_val; int error; 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_array = prop->pointer; + ref_args = &ref_array[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; @@ -510,7 +543,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 c592c286e3394..19b9dcc322763 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 { @@ -223,6 +224,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. @@ -260,14 +275,20 @@ struct property_entry { #define __PROPERTY_ENTRY_ELEMENT_SIZE(_elem_) \ sizeof(((struct property_entry *)NULL)->value._elem_[0]) -#define __PROPERTY_ENTRY_ARRAY_LEN(_name_, _elem_, _Type_, _val_, _len_)\ +#define __PROPERTY_ENTRY_ARRAY_ELSIZE_LEN(_name_, _elsize_, _Type_, \ + _val_, _len_) \ (struct property_entry) { \ .name = _name_, \ - .length = (_len_) * __PROPERTY_ENTRY_ELEMENT_SIZE(_elem_), \ + .length = (_len_) * (_elsize_), \ .type = DEV_PROP_##_Type_, \ { .pointer = _val_ }, \ } +#define __PROPERTY_ENTRY_ARRAY_LEN(_name_, _elem_, _Type_, _val_, _len_)\ + __PROPERTY_ENTRY_ARRAY_ELSIZE_LEN(_name_, \ + __PROPERTY_ENTRY_ELEMENT_SIZE(_elem_), \ + _Type_, _val_, _len_) + #define PROPERTY_ENTRY_U8_ARRAY_LEN(_name_, _val_, _len_) \ __PROPERTY_ENTRY_ARRAY_LEN(_name_, u8_data, U8, _val_, _len_) #define PROPERTY_ENTRY_U16_ARRAY_LEN(_name_, _val_, _len_) \ @@ -278,6 +299,10 @@ 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_ELSIZE_LEN(_name_, \ + sizeof(struct software_node_ref_args), \ + REF, _val_, _len_) #define PROPERTY_ENTRY_U8_ARRAY(_name_, _val_) \ PROPERTY_ENTRY_U8_ARRAY_LEN(_name_, _val_, ARRAY_SIZE(_val_)) @@ -289,6 +314,8 @@ struct property_entry { PROPERTY_ENTRY_U64_ARRAY_LEN(_name_, _val_, ARRAY_SIZE(_val_)) #define PROPERTY_ENTRY_STRING_ARRAY(_name_, _val_) \ PROPERTY_ENTRY_STRING_ARRAY_LEN(_name_, _val_, ARRAY_SIZE(_val_)) +#define PROPERTY_ENTRY_REF_ARRAY(_name_, _val_) \ + PROPERTY_ENTRY_REF_ARRAY_LEN(_name_, _val_, ARRAY_SIZE(_val_)) #define __PROPERTY_ENTRY_ELEMENT(_name_, _elem_, _Type_, _val_) \ (struct property_entry) { \ @@ -316,6 +343,18 @@ struct property_entry { .is_inline = true, \ } +#define PROPERTY_ENTRY_REF(_name_, _ref_, ...) \ +(struct property_entry) { \ + .name = _name_, \ + .length = sizeof(struct software_node_ref_args), \ + .type = DEV_PROP_REF, \ + { .pointer = &(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); @@ -379,20 +418,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.24.0.rc1.363.gb1bccd3e3d-goog ^ permalink raw reply related [flat|nested] 28+ messages in thread
[parent not found: <CGME20201109170241eucas1p14c2156334d8c6ef15d52664fa4776f41@eucas1p1.samsung.com>]
* Re: [PATCH v8 3/6] software node: implement reference properties [not found] ` <CGME20201109170241eucas1p14c2156334d8c6ef15d52664fa4776f41@eucas1p1.samsung.com> @ 2020-11-09 17:02 ` Lukasz Stelmach 2020-11-09 17:24 ` Andy Shevchenko 0 siblings, 1 reply; 28+ messages in thread From: Lukasz Stelmach @ 2020-11-09 17:02 UTC (permalink / raw) To: Dmitry Torokhov Cc: Rafael J. Wysocki, Heikki Krogerus, Andy Shevchenko, Mika Westerberg, Linus Walleij, Ard Biesheuvel, linux-acpi, linux-kernel, platform-driver-x86, Marek Szyprowski, Bartlomiej Zolnierkiewicz [-- Attachment #1: Type: text/plain, Size: 1712 bytes --] It was <2019-11-07 czw 20:22>, when Dmitry Torokhov wrote: > 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> > --- I am writing a piece that needs to provide a list of gpios to a diriver. The above example looks like what I need. At the moment the driver gets the list from fwnode/of_node. The list contain references to phandles which get resolved and and the driver ends up with a bunch of gpio descriptors. Great. This example looks nice but does the code that reads the reference from the gpios property and returns a gpiod actually exist? If it doesn't, I am willing to write it. At first glance it makes more sense to me to pass (struct gpiod_lookup *) instead of (struct software_node *) and make gpiolib's gpiod_find() accept lookup tables as parameter instead of searching the gpio_lookup_list? Or do you think such temporary table should be assembled from the above structure and then used in gpiod_find()? Any other suggestions on how to get a bunch of gpios (the description for gpios is available in the devicetree) for a device described with a software nodes? Kind regards, -- Łukasz Stelmach Samsung R&D Institute Poland Samsung Electronics [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 487 bytes --] ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v8 3/6] software node: implement reference properties 2020-11-09 17:02 ` Lukasz Stelmach @ 2020-11-09 17:24 ` Andy Shevchenko [not found] ` <CGME20201109181851eucas1p241de8938e399c0b603c764593b057c41@eucas1p2.samsung.com> 0 siblings, 1 reply; 28+ messages in thread From: Andy Shevchenko @ 2020-11-09 17:24 UTC (permalink / raw) To: Lukasz Stelmach Cc: Dmitry Torokhov, Rafael J. Wysocki, Heikki Krogerus, Mika Westerberg, Linus Walleij, Ard Biesheuvel, linux-acpi, linux-kernel, platform-driver-x86, Marek Szyprowski, Bartlomiej Zolnierkiewicz On Mon, Nov 09, 2020 at 06:02:29PM +0100, Lukasz Stelmach wrote: > It was <2019-11-07 czw 20:22>, when Dmitry Torokhov wrote: > > 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> > > --- > > I am writing a piece that needs to provide a list of gpios to a > diriver. The above example looks like what I need. Nope. It mustn't be used for GPIOs or PWMs or whatever that either should come via lookup tables or corresponding firmware interface. > At the moment the driver gets the list from fwnode/of_node. The list > contain references to phandles which get resolved and and the driver > ends up with a bunch of gpio descriptors. Great. > > This example looks nice but does the code that reads the reference from > the gpios property and returns a gpiod actually exist? If it doesn't, I > am willing to write it. > > At first glance it makes more sense to me to pass (struct gpiod_lookup > *) instead of (struct software_node *) and make gpiolib's gpiod_find() > accept lookup tables as parameter instead of searching the > gpio_lookup_list? Or do you think such temporary table should be > assembled from the above structure and then used in gpiod_find()? > > Any other suggestions on how to get a bunch of gpios (the description > for gpios is available in the devicetree) for a device described with a > software nodes? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 28+ messages in thread
[parent not found: <CGME20201109181851eucas1p241de8938e399c0b603c764593b057c41@eucas1p2.samsung.com>]
* Re: [PATCH v8 3/6] software node: implement reference properties [not found] ` <CGME20201109181851eucas1p241de8938e399c0b603c764593b057c41@eucas1p2.samsung.com> @ 2020-11-09 18:18 ` Lukasz Stelmach 2020-11-09 18:53 ` Dmitry Torokhov 2020-11-09 19:02 ` Andy Shevchenko 0 siblings, 2 replies; 28+ messages in thread From: Lukasz Stelmach @ 2020-11-09 18:18 UTC (permalink / raw) To: Andy Shevchenko Cc: Dmitry Torokhov, Rafael J. Wysocki, Heikki Krogerus, Mika Westerberg, Linus Walleij, Ard Biesheuvel, linux-acpi, linux-kernel, platform-driver-x86, Marek Szyprowski, Bartlomiej Zolnierkiewicz [-- Attachment #1: Type: text/plain, Size: 3126 bytes --] It was <2020-11-09 pon 19:24>, when Andy Shevchenko wrote: > On Mon, Nov 09, 2020 at 06:02:29PM +0100, Lukasz Stelmach wrote: >> It was <2019-11-07 czw 20:22>, when Dmitry Torokhov wrote: >> > 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> >> > --- >> >> I am writing a piece that needs to provide a list of gpios to a >> diriver. The above example looks like what I need. > > Nope. > > It mustn't be used for GPIOs or PWMs or whatever that either should come via > lookup tables or corresponding firmware interface. > May I ask why? I've read commit descriptions for drivers/base/swnode.c and the discussion on lkml and I understand software nodes as a way to provide (synthesize) a description for a device that is missing a description in the firmware. Another use case seems to be to replace (in the long run) platform data. That is what I am trying to use it for. I want my device to be configured with either DT or software_nodes created at run time with configfs. My device is going to use GPIOs described in the DT and it is going to be configured via configfs at run time. I could use platform_data to pass structures from configfs but software nodes would let me save some code in the device driver and use the same paths for both static (DT) and dynamic (configfs) configuration. Probably I have missed something and I will be greatful, if you tell me where I can find more information about software nodes. There are few users in the kernel and it isn't obvious for me how to use software nodes properly. >> At the moment the driver gets the list from fwnode/of_node. The list >> contain references to phandles which get resolved and and the driver >> ends up with a bunch of gpio descriptors. Great. >> >> This example looks nice but does the code that reads the reference from >> the gpios property and returns a gpiod actually exist? If it doesn't, I >> am willing to write it. >> >> At first glance it makes more sense to me to pass (struct gpiod_lookup >> *) instead of (struct software_node *) and make gpiolib's gpiod_find() >> accept lookup tables as parameter instead of searching the >> gpio_lookup_list? Or do you think such temporary table should be >> assembled from the above structure and then used in gpiod_find()? >> >> Any other suggestions on how to get a bunch of gpios (the description >> for gpios is available in the devicetree) for a device described with a >> software nodes? Kind regards, -- Łukasz Stelmach Samsung R&D Institute Poland Samsung Electronics [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 487 bytes --] ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v8 3/6] software node: implement reference properties 2020-11-09 18:18 ` Lukasz Stelmach @ 2020-11-09 18:53 ` Dmitry Torokhov 2020-11-09 19:05 ` Andy Shevchenko [not found] ` <CGME20201109195504eucas1p19d493c947d8752e39c26202cf0978fc0@eucas1p1.samsung.com> 2020-11-09 19:02 ` Andy Shevchenko 1 sibling, 2 replies; 28+ messages in thread From: Dmitry Torokhov @ 2020-11-09 18:53 UTC (permalink / raw) To: Lukasz Stelmach Cc: Andy Shevchenko, Rafael J. Wysocki, Heikki Krogerus, Mika Westerberg, Linus Walleij, Ard Biesheuvel, linux-acpi, linux-kernel, platform-driver-x86, Marek Szyprowski, Bartlomiej Zolnierkiewicz On Mon, Nov 09, 2020 at 07:18:37PM +0100, Lukasz Stelmach wrote: > It was <2020-11-09 pon 19:24>, when Andy Shevchenko wrote: > > On Mon, Nov 09, 2020 at 06:02:29PM +0100, Lukasz Stelmach wrote: > >> It was <2019-11-07 czw 20:22>, when Dmitry Torokhov wrote: > >> > 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> > >> > --- > >> > >> I am writing a piece that needs to provide a list of gpios to a > >> diriver. The above example looks like what I need. > > > > Nope. > > > > It mustn't be used for GPIOs or PWMs or whatever that either should come via > > lookup tables or corresponding firmware interface. > > > > May I ask why? I've read commit descriptions for drivers/base/swnode.c > and the discussion on lkml and I understand software nodes as a way to > provide (synthesize) a description for a device that is missing a > description in the firmware. Another use case seems to be to replace (in > the long run) platform data. That is what I am trying to use it for. > > I want my device to be configured with either DT or software_nodes > created at run time with configfs. My device is going to use GPIOs > described in the DT and it is going to be configured via configfs at run > time. I could use platform_data to pass structures from configfs but > software nodes would let me save some code in the device driver and use > the same paths for both static (DT) and dynamic (configfs) > configuration. > > Probably I have missed something and I will be greatful, if you tell me > where I can find more information about software nodes. There are few > users in the kernel and it isn't obvious for me how to use software > nodes properly. Yeah, I disagree with Andy here. The lookup tables are a crutch that we have until GPIO and PWM a taught to support software nodes (I need to resurrect my patch series for GPIO, if you have time to test that would be awesome). Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v8 3/6] software node: implement reference properties 2020-11-09 18:53 ` Dmitry Torokhov @ 2020-11-09 19:05 ` Andy Shevchenko 2020-11-10 12:39 ` Heikki Krogerus [not found] ` <CGME20201109195504eucas1p19d493c947d8752e39c26202cf0978fc0@eucas1p1.samsung.com> 1 sibling, 1 reply; 28+ messages in thread From: Andy Shevchenko @ 2020-11-09 19:05 UTC (permalink / raw) To: Dmitry Torokhov Cc: Lukasz Stelmach, Rafael J. Wysocki, Heikki Krogerus, Mika Westerberg, Linus Walleij, Ard Biesheuvel, linux-acpi, linux-kernel, platform-driver-x86, Marek Szyprowski, Bartlomiej Zolnierkiewicz On Mon, Nov 09, 2020 at 10:53:05AM -0800, Dmitry Torokhov wrote: > On Mon, Nov 09, 2020 at 07:18:37PM +0100, Lukasz Stelmach wrote: > > It was <2020-11-09 pon 19:24>, when Andy Shevchenko wrote: ... > > Probably I have missed something and I will be greatful, if you tell me > > where I can find more information about software nodes. There are few > > users in the kernel and it isn't obvious for me how to use software > > nodes properly. > > Yeah, I disagree with Andy here. The lookup tables are a crutch that we > have until GPIO and PWM a taught to support software nodes (I need to > resurrect my patch series for GPIO, if you have time to test that would > be awesome). We don't have support for list of fwnodes, this will probably break things where swnode is already exist. I think Heikki may send a documentation patch to clarify when swnodes can and can't be used. Maybe I'm mistaken and above is a good use case by design. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v8 3/6] software node: implement reference properties 2020-11-09 19:05 ` Andy Shevchenko @ 2020-11-10 12:39 ` Heikki Krogerus 2020-11-10 12:46 ` Rafael J. Wysocki 0 siblings, 1 reply; 28+ messages in thread From: Heikki Krogerus @ 2020-11-10 12:39 UTC (permalink / raw) To: Andy Shevchenko Cc: Dmitry Torokhov, Lukasz Stelmach, Rafael J. Wysocki, Mika Westerberg, Linus Walleij, Ard Biesheuvel, linux-acpi, linux-kernel, platform-driver-x86, Marek Szyprowski, Bartlomiej Zolnierkiewicz On Mon, Nov 09, 2020 at 09:05:51PM +0200, Andy Shevchenko wrote: > On Mon, Nov 09, 2020 at 10:53:05AM -0800, Dmitry Torokhov wrote: > > On Mon, Nov 09, 2020 at 07:18:37PM +0100, Lukasz Stelmach wrote: > > > It was <2020-11-09 pon 19:24>, when Andy Shevchenko wrote: > > ... > > > > Probably I have missed something and I will be greatful, if you tell me > > > where I can find more information about software nodes. There are few > > > users in the kernel and it isn't obvious for me how to use software > > > nodes properly. > > > > Yeah, I disagree with Andy here. The lookup tables are a crutch that we > > have until GPIO and PWM a taught to support software nodes (I need to > > resurrect my patch series for GPIO, if you have time to test that would > > be awesome). > > We don't have support for list of fwnodes, this will probably break things > where swnode is already exist. > > I think Heikki may send a documentation patch to clarify when swnodes can and > can't be used. Maybe I'm mistaken and above is a good use case by design. Yeah, the problem is that I'm not sure that we can limit the swnodes for any specific purpose, or dictate very strictly how they are used with other possible fwnodes. Right now I'm thinking we should simply not talk about the relationship a software node should have or can have with other fwnodes (regardless of their type) in the swnode documentation. Br, -- heikki ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v8 3/6] software node: implement reference properties 2020-11-10 12:39 ` Heikki Krogerus @ 2020-11-10 12:46 ` Rafael J. Wysocki 0 siblings, 0 replies; 28+ messages in thread From: Rafael J. Wysocki @ 2020-11-10 12:46 UTC (permalink / raw) To: Heikki Krogerus Cc: Andy Shevchenko, Dmitry Torokhov, Lukasz Stelmach, Rafael J. Wysocki, Mika Westerberg, Linus Walleij, Ard Biesheuvel, ACPI Devel Maling List, Linux Kernel Mailing List, Platform Driver, Marek Szyprowski, Bartlomiej Zolnierkiewicz On Tue, Nov 10, 2020 at 1:39 PM Heikki Krogerus <heikki.krogerus@linux.intel.com> wrote: > > On Mon, Nov 09, 2020 at 09:05:51PM +0200, Andy Shevchenko wrote: > > On Mon, Nov 09, 2020 at 10:53:05AM -0800, Dmitry Torokhov wrote: > > > On Mon, Nov 09, 2020 at 07:18:37PM +0100, Lukasz Stelmach wrote: > > > > It was <2020-11-09 pon 19:24>, when Andy Shevchenko wrote: > > > > ... > > > > > > Probably I have missed something and I will be greatful, if you tell me > > > > where I can find more information about software nodes. There are few > > > > users in the kernel and it isn't obvious for me how to use software > > > > nodes properly. > > > > > > Yeah, I disagree with Andy here. The lookup tables are a crutch that we > > > have until GPIO and PWM a taught to support software nodes (I need to > > > resurrect my patch series for GPIO, if you have time to test that would > > > be awesome). > > > > We don't have support for list of fwnodes, this will probably break things > > where swnode is already exist. > > > > I think Heikki may send a documentation patch to clarify when swnodes can and > > can't be used. Maybe I'm mistaken and above is a good use case by design. > > Yeah, the problem is that I'm not sure that we can limit the swnodes > for any specific purpose, or dictate very strictly how they are used > with other possible fwnodes. Generally agreed, but if there are known problems, they need to be documented at least for the time being until they are addressed. > Right now I'm thinking we should simply not talk about the > relationship a software node should have or can have with other > fwnodes (regardless of their type) in the swnode documentation. This sounds reasonable to me, with the above exception. ^ permalink raw reply [flat|nested] 28+ messages in thread
[parent not found: <CGME20201109195504eucas1p19d493c947d8752e39c26202cf0978fc0@eucas1p1.samsung.com>]
* Re: [PATCH v8 3/6] software node: implement reference properties [not found] ` <CGME20201109195504eucas1p19d493c947d8752e39c26202cf0978fc0@eucas1p1.samsung.com> @ 2020-11-09 19:54 ` Lukasz Stelmach 0 siblings, 0 replies; 28+ messages in thread From: Lukasz Stelmach @ 2020-11-09 19:54 UTC (permalink / raw) To: Dmitry Torokhov Cc: Andy Shevchenko, Rafael J. Wysocki, Heikki Krogerus, Mika Westerberg, Linus Walleij, Ard Biesheuvel, linux-acpi, linux-kernel, platform-driver-x86, Marek Szyprowski, Bartlomiej Zolnierkiewicz [-- Attachment #1: Type: text/plain, Size: 2802 bytes --] It was <2020-11-09 pon 10:53>, when Dmitry Torokhov wrote: > On Mon, Nov 09, 2020 at 07:18:37PM +0100, Lukasz Stelmach wrote: >> It was <2020-11-09 pon 19:24>, when Andy Shevchenko wrote: >> > On Mon, Nov 09, 2020 at 06:02:29PM +0100, Lukasz Stelmach wrote: >> >> It was <2019-11-07 czw 20:22>, when Dmitry Torokhov wrote: >> >> > 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> >> >> > --- >> >> >> >> I am writing a piece that needs to provide a list of gpios to a >> >> diriver. The above example looks like what I need. >> > >> > Nope. >> > >> > It mustn't be used for GPIOs or PWMs or whatever that either should come via >> > lookup tables or corresponding firmware interface. >> > >> >> May I ask why? I've read commit descriptions for drivers/base/swnode.c >> and the discussion on lkml and I understand software nodes as a way to >> provide (synthesize) a description for a device that is missing a >> description in the firmware. Another use case seems to be to replace (in >> the long run) platform data. That is what I am trying to use it for. >> >> I want my device to be configured with either DT or software_nodes >> created at run time with configfs. My device is going to use GPIOs >> described in the DT and it is going to be configured via configfs at run >> time. I could use platform_data to pass structures from configfs but >> software nodes would let me save some code in the device driver and use >> the same paths for both static (DT) and dynamic (configfs) >> configuration. >> >> Probably I have missed something and I will be greatful, if you tell me >> where I can find more information about software nodes. There are few >> users in the kernel and it isn't obvious for me how to use software >> nodes properly. > > Yeah, I disagree with Andy here. The lookup tables are a crutch that we > have until GPIO and PWM a taught to support software nodes (I need to > resurrect my patch series for GPIO, if you have time to test that would > be awesome). It will be great to help. I am not sure if I have means to test PWMs, but I can do GPIOs. -- Łukasz Stelmach Samsung R&D Institute Poland Samsung Electronics [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 487 bytes --] ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v8 3/6] software node: implement reference properties 2020-11-09 18:18 ` Lukasz Stelmach 2020-11-09 18:53 ` Dmitry Torokhov @ 2020-11-09 19:02 ` Andy Shevchenko [not found] ` <CGME20201109194725eucas1p2cc9357486879a14b2ad2f6ef968ff4b2@eucas1p2.samsung.com> 1 sibling, 1 reply; 28+ messages in thread From: Andy Shevchenko @ 2020-11-09 19:02 UTC (permalink / raw) To: Lukasz Stelmach Cc: Dmitry Torokhov, Rafael J. Wysocki, Heikki Krogerus, Mika Westerberg, Linus Walleij, Ard Biesheuvel, linux-acpi, linux-kernel, platform-driver-x86, Marek Szyprowski, Bartlomiej Zolnierkiewicz On Mon, Nov 09, 2020 at 07:18:37PM +0100, Lukasz Stelmach wrote: > It was <2020-11-09 pon 19:24>, when Andy Shevchenko wrote: > > On Mon, Nov 09, 2020 at 06:02:29PM +0100, Lukasz Stelmach wrote: > >> It was <2019-11-07 czw 20:22>, when Dmitry Torokhov wrote: ... > >> I am writing a piece that needs to provide a list of gpios to a > >> diriver. The above example looks like what I need. > > > > Nope. > > > > It mustn't be used for GPIOs or PWMs or whatever that either should come via > > lookup tables or corresponding firmware interface. > > May I ask why? I've read commit descriptions for drivers/base/swnode.c > and the discussion on lkml and I understand software nodes as a way to > provide (synthesize) a description for a device that is missing a > description in the firmware. Another use case seems to be to replace (in > the long run) platform data. That is what I am trying to use it for. Yes. Both are correct. They are simply not applicable for everything (it's not a silver bullet). > I want my device to be configured with either DT or software_nodes > created at run time with configfs. Okay. > My device is going to use GPIOs > described in the DT and it is going to be configured via configfs at run > time. How is this related to swnodes? Create GPIO lookup table. > I could use platform_data to pass structures from configfs but > software nodes would let me save some code in the device driver and use > the same paths for both static (DT) and dynamic (configfs) > configuration. > > Probably I have missed something and I will be greatful, if you tell me > where I can find more information about software nodes. There are few > users in the kernel and it isn't obvious for me how to use software > nodes properly. gpiod_add_lookup_table(). > >> At the moment the driver gets the list from fwnode/of_node. The list > >> contain references to phandles which get resolved and and the driver > >> ends up with a bunch of gpio descriptors. Great. > >> > >> This example looks nice but does the code that reads the reference from > >> the gpios property and returns a gpiod actually exist? If it doesn't, I > >> am willing to write it. > >> > >> At first glance it makes more sense to me to pass (struct gpiod_lookup > >> *) instead of (struct software_node *) and make gpiolib's gpiod_find() > >> accept lookup tables as parameter instead of searching the > >> gpio_lookup_list? Or do you think such temporary table should be > >> assembled from the above structure and then used in gpiod_find()? > >> > >> Any other suggestions on how to get a bunch of gpios (the description > >> for gpios is available in the devicetree) for a device described with a > >> software nodes? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 28+ messages in thread
[parent not found: <CGME20201109194725eucas1p2cc9357486879a14b2ad2f6ef968ff4b2@eucas1p2.samsung.com>]
* Re: [PATCH v8 3/6] software node: implement reference properties [not found] ` <CGME20201109194725eucas1p2cc9357486879a14b2ad2f6ef968ff4b2@eucas1p2.samsung.com> @ 2020-11-09 19:47 ` Lukasz Stelmach 2020-11-09 21:20 ` Andy Shevchenko 0 siblings, 1 reply; 28+ messages in thread From: Lukasz Stelmach @ 2020-11-09 19:47 UTC (permalink / raw) To: Andy Shevchenko Cc: Dmitry Torokhov, Rafael J. Wysocki, Heikki Krogerus, Mika Westerberg, Linus Walleij, Ard Biesheuvel, linux-acpi, linux-kernel, platform-driver-x86, Marek Szyprowski, Bartlomiej Zolnierkiewicz [-- Attachment #1: Type: text/plain, Size: 4221 bytes --] It was <2020-11-09 pon 21:02>, when Andy Shevchenko wrote: > On Mon, Nov 09, 2020 at 07:18:37PM +0100, Lukasz Stelmach wrote: >> It was <2020-11-09 pon 19:24>, when Andy Shevchenko wrote: >> > On Mon, Nov 09, 2020 at 06:02:29PM +0100, Lukasz Stelmach wrote: >> >> It was <2019-11-07 czw 20:22>, when Dmitry Torokhov wrote: > > ... > >> >> I am writing a piece that needs to provide a list of gpios to a >> >> diriver. The above example looks like what I need. >> > >> > Nope. >> > >> > It mustn't be used for GPIOs or PWMs or whatever that either should come via >> > lookup tables or corresponding firmware interface. >> >> May I ask why? I've read commit descriptions for drivers/base/swnode.c >> and the discussion on lkml and I understand software nodes as a way to >> provide (synthesize) a description for a device that is missing a >> description in the firmware. Another use case seems to be to replace (in >> the long run) platform data. That is what I am trying to use it for. > > Yes. Both are correct. They are simply not applicable for everything > (it's not a silver bullet). > >> I want my device to be configured with either DT or software_nodes >> created at run time with configfs. > > Okay. > >> My device is going to use GPIOs >> described in the DT and it is going to be configured via configfs at run >> time. > > How is this related to swnodes? I thought I should mention it, because as far as I can tel mixing and merging information from different fwnode backends seems to be tricky if supported at all. > Create GPIO lookup table. > >> I could use platform_data to pass structures from configfs but >> software nodes would let me save some code in the device driver and use >> the same paths for both static (DT) and dynamic (configfs) >> configuration. >> >> Probably I have missed something and I will be greatful, if you tell me >> where I can find more information about software nodes. There are few >> users in the kernel and it isn't obvious for me how to use software >> nodes properly. > > gpiod_add_lookup_table(). > Yes, that is exactly what my POC code does now. But having a lookup table together with the rest of the device structures has several advantages. 1) The device may be hotpluggable and there is no gpiod_remove_lookup_table(). 2) Having the lookup table allocated and managed together with the rest of the device seems like a better way to go than on gpio_lookup_list. 3) As of now I've got a minor issue with device naming. I need to set dev_id of the table before the device is ready and only after it is ready, its name is set (in the hotpluggable use case). 4) Because no other devices would use this lookup table "publishing" it rather than keeping together with the device seems at least slightly odd. When the lookup table is attached to the devices and can be passed around the final lookup can be done with a function like static struct gpio_desc *gpiod_find_from_table(struct device *dev, const char *con_id, unsigned int idx, unsigned long *flags, struct gpiod_lookup *table) >>>> At the moment the driver gets the list from fwnode/of_node. The list >>>> contain references to phandles which get resolved and and the driver >>>> ends up with a bunch of gpio descriptors. Great. >>>> >>>> This example looks nice but does the code that reads the reference from >>>> the gpios property and returns a gpiod actually exist? If it doesn't, I >>>> am willing to write it. >>>> >>>> At first glance it makes more sense to me to pass (struct gpiod_lookup >>>> *) instead of (struct software_node *) and make gpiolib's gpiod_find() >>>> accept lookup tables as parameter instead of searching the >>>> gpio_lookup_list? Or do you think such temporary table should be >>>> assembled from the above structure and then used in gpiod_find()? >>>> >>>> Any other suggestions on how to get a bunch of gpios (the description >>>> for gpios is available in the devicetree) for a device described with a >>>> software nodes? -- Łukasz Stelmach Samsung R&D Institute Poland Samsung Electronics [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 487 bytes --] ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v8 3/6] software node: implement reference properties 2020-11-09 19:47 ` Lukasz Stelmach @ 2020-11-09 21:20 ` Andy Shevchenko 0 siblings, 0 replies; 28+ messages in thread From: Andy Shevchenko @ 2020-11-09 21:20 UTC (permalink / raw) To: Lukasz Stelmach Cc: Dmitry Torokhov, Rafael J. Wysocki, Heikki Krogerus, Mika Westerberg, Linus Walleij, Ard Biesheuvel, linux-acpi, linux-kernel, platform-driver-x86, Marek Szyprowski, Bartlomiej Zolnierkiewicz On Mon, Nov 09, 2020 at 08:47:14PM +0100, Lukasz Stelmach wrote: > It was <2020-11-09 pon 21:02>, when Andy Shevchenko wrote: > > On Mon, Nov 09, 2020 at 07:18:37PM +0100, Lukasz Stelmach wrote: > >> It was <2020-11-09 pon 19:24>, when Andy Shevchenko wrote: > >> > On Mon, Nov 09, 2020 at 06:02:29PM +0100, Lukasz Stelmach wrote: > >> >> It was <2019-11-07 czw 20:22>, when Dmitry Torokhov wrote: ... > > Create GPIO lookup table. > > > >> I could use platform_data to pass structures from configfs but > >> software nodes would let me save some code in the device driver and use > >> the same paths for both static (DT) and dynamic (configfs) > >> configuration. > >> > >> Probably I have missed something and I will be greatful, if you tell me > >> where I can find more information about software nodes. There are few > >> users in the kernel and it isn't obvious for me how to use software > >> nodes properly. > > > > gpiod_add_lookup_table(). > > > > Yes, that is exactly what my POC code does now. But having a lookup > table together with the rest of the device structures has several > advantages. > > 1) The device may be hotpluggable and there is no > gpiod_remove_lookup_table(). % git grep -n -w gpiod_remove_lookup_table Or I did get it wrong? Did you mean that the removal is not being called? > 2) Having the lookup table allocated and managed together with the rest > of the device seems like a better way to go than on gpio_lookup_list. Nice, what are you going to do with the rest of lookup tables (PWM, regulators, etc)? If you convert, convert them all at least. > 3) As of now I've got a minor issue with device naming. I need to set > dev_id of the table before the device is ready and only after it is > ready, its name is set (in the hotpluggable use case). Hotpluggable devices are very much supported by ACPI assistance. DT I have heard has overlays. What's the issue? > 4) Because no other devices would use this lookup table "publishing" it > rather than keeping together with the device seems at least slightly > odd. > > When the lookup table is attached to the devices and can be passed > around the final lookup can be done with a function like > > static struct gpio_desc *gpiod_find_from_table(struct device *dev, > const char *con_id, unsigned int idx, > unsigned long *flags, struct gpiod_lookup *table) Something sounds fishy about your case. Why do you need to have board code / platform data in the first place? Sorry, but I didn't get why you should reconstruct DT (or ACPI) at run-time without using proper framework / feature (overlays)? > >>>> At the moment the driver gets the list from fwnode/of_node. The list > >>>> contain references to phandles which get resolved and and the driver > >>>> ends up with a bunch of gpio descriptors. Great. > >>>> > >>>> This example looks nice but does the code that reads the reference from > >>>> the gpios property and returns a gpiod actually exist? If it doesn't, I > >>>> am willing to write it. > >>>> > >>>> At first glance it makes more sense to me to pass (struct gpiod_lookup > >>>> *) instead of (struct software_node *) and make gpiolib's gpiod_find() > >>>> accept lookup tables as parameter instead of searching the > >>>> gpio_lookup_list? Or do you think such temporary table should be > >>>> assembled from the above structure and then used in gpiod_find()? > >>>> > >>>> Any other suggestions on how to get a bunch of gpios (the description > >>>> for gpios is available in the devicetree) for a device described with a > >>>> software nodes? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v8 4/6] platform/x86: intel_cht_int33fe: use inline reference properties 2019-11-08 4:22 [PATCH v8 0/6] software node: add support for reference properties Dmitry Torokhov ` (2 preceding siblings ...) 2019-11-08 4:22 ` [PATCH v8 3/6] software node: implement reference properties Dmitry Torokhov @ 2019-11-08 4:22 ` Dmitry Torokhov 2019-11-08 4:22 ` [PATCH v8 5/6] software node: remove separate handling of references Dmitry Torokhov 2019-11-08 4:22 ` [PATCH v8 6/6] software node: add basic tests for property entries Dmitry Torokhov 5 siblings, 0 replies; 28+ messages in thread From: Dmitry Torokhov @ 2019-11-08 4:22 UTC (permalink / raw) To: Rafael J. Wysocki, Heikki Krogerus Cc: Andy Shevchenko, Mika Westerberg, Linus Walleij, Ard Biesheuvel, linux-acpi, 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. Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- .../platform/x86/intel_cht_int33fe_typec.c | 81 ++++++++++--------- 1 file changed, 41 insertions(+), 40 deletions(-) diff --git a/drivers/platform/x86/intel_cht_int33fe_typec.c b/drivers/platform/x86/intel_cht_int33fe_typec.c index 2d097fc2dd465..04138215956bf 100644 --- a/drivers/platform/x86/intel_cht_int33fe_typec.c +++ b/drivers/platform/x86/intel_cht_int33fe_typec.c @@ -36,30 +36,6 @@ enum { INT33FE_NODE_MAX, }; -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 @@ -95,8 +71,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), { } }; @@ -112,6 +98,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"), @@ -119,15 +107,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 }, { } }; @@ -163,9 +157,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) { @@ -177,25 +172,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.24.0.rc1.363.gb1bccd3e3d-goog ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v8 5/6] software node: remove separate handling of references 2019-11-08 4:22 [PATCH v8 0/6] software node: add support for reference properties Dmitry Torokhov ` (3 preceding siblings ...) 2019-11-08 4:22 ` [PATCH v8 4/6] platform/x86: intel_cht_int33fe: use inline " Dmitry Torokhov @ 2019-11-08 4:22 ` Dmitry Torokhov 2019-11-08 4:22 ` [PATCH v8 6/6] software node: add basic tests for property entries Dmitry Torokhov 5 siblings, 0 replies; 28+ messages in thread From: Dmitry Torokhov @ 2019-11-08 4:22 UTC (permalink / raw) To: Rafael J. Wysocki, Heikki Krogerus Cc: Andy Shevchenko, Mika Westerberg, Linus Walleij, Ard Biesheuvel, linux-acpi, 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 | 48 +++++++++++++++------------------------- include/linux/property.h | 14 ------------ 2 files changed, 18 insertions(+), 44 deletions(-) diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c index 604d7327bba79..0b081dee1e95c 100644 --- a/drivers/base/swnode.c +++ b/drivers/base/swnode.c @@ -479,9 +479,8 @@ 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_array; - const struct software_node_ref_args *ref_args; + const struct software_node_ref_args *ref; const struct property_entry *prop; struct fwnode_handle *refnode; u32 nargs_prop_val; @@ -492,37 +491,26 @@ 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; - - ref_array = prop->pointer; - ref_args = &ref_array[index]; - } else { - if (!swnode->node->references) - return -ENOENT; + if (!prop) + 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_array = prop->pointer; + ref = &ref_array[index]; - refnode = software_node_fwnode(ref_args->node); + refnode = software_node_fwnode(ref->node); if (!refnode) return -ENOENT; @@ -543,7 +531,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 19b9dcc322763..b28c81af7bb68 100644 --- a/include/linux/property.h +++ b/include/linux/property.h @@ -418,30 +418,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.24.0.rc1.363.gb1bccd3e3d-goog ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v8 6/6] software node: add basic tests for property entries 2019-11-08 4:22 [PATCH v8 0/6] software node: add support for reference properties Dmitry Torokhov ` (4 preceding siblings ...) 2019-11-08 4:22 ` [PATCH v8 5/6] software node: remove separate handling of references Dmitry Torokhov @ 2019-11-08 4:22 ` Dmitry Torokhov 5 siblings, 0 replies; 28+ messages in thread From: Dmitry Torokhov @ 2019-11-08 4:22 UTC (permalink / raw) To: Rafael J. Wysocki, Heikki Krogerus Cc: Andy Shevchenko, Mika Westerberg, Linus Walleij, Ard Biesheuvel, linux-acpi, linux-kernel, platform-driver-x86 This adds tests for creating software nodes with properties supplied by PROPERTY_ENTRY_XXX() macros and fetching and validating data from said nodes/properties. We are using KUnit framework for the tests. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- drivers/base/test/Makefile | 2 + drivers/base/test/property-entry-test.c | 472 ++++++++++++++++++++++++ 2 files changed, 474 insertions(+) create mode 100644 drivers/base/test/property-entry-test.c diff --git a/drivers/base/test/Makefile b/drivers/base/test/Makefile index 0f1f7277a0139..22143102e5d21 100644 --- a/drivers/base/test/Makefile +++ b/drivers/base/test/Makefile @@ -1,2 +1,4 @@ # SPDX-License-Identifier: GPL-2.0 obj-$(CONFIG_TEST_ASYNC_DRIVER_PROBE) += test_async_driver_probe.o + +obj-$(CONFIG_KUNIT) += property-entry-test.o diff --git a/drivers/base/test/property-entry-test.c b/drivers/base/test/property-entry-test.c new file mode 100644 index 0000000000000..d523a3d9caeda --- /dev/null +++ b/drivers/base/test/property-entry-test.c @@ -0,0 +1,472 @@ +// SPDX-License-Identifier: GPL-2.0 +// Unit tests for property entries API +// +// Copyright 2019 Google LLC. + +#include <kunit/test.h> +#include <linux/property.h> +#include <linux/types.h> + +static void pe_test_uints(struct kunit *test) +{ + const struct property_entry entries[] = { + PROPERTY_ENTRY_U8("prop-u8", 8), + PROPERTY_ENTRY_U16("prop-u16", 16), + PROPERTY_ENTRY_U32("prop-u32", 32), + PROPERTY_ENTRY_U64("prop-u64", 64), + { } + }; + + struct fwnode_handle *node; + u8 val_u8, array_u8[2]; + u16 val_u16, array_u16[2]; + u32 val_u32, array_u32[2]; + u64 val_u64, array_u64[2]; + int error; + + node = fwnode_create_software_node(entries, NULL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, node); + + error = fwnode_property_read_u8(node, "prop-u8", &val_u8); + KUNIT_EXPECT_EQ(test, error, 0); + KUNIT_EXPECT_EQ(test, (int)val_u8, 8); + + error = fwnode_property_read_u8_array(node, "prop-u8", array_u8, 1); + KUNIT_EXPECT_EQ(test, error, 0); + KUNIT_EXPECT_EQ(test, (int)array_u8[0], 8); + + error = fwnode_property_read_u8_array(node, "prop-u8", array_u8, 2); + KUNIT_EXPECT_NE(test, error, 0); + + error = fwnode_property_read_u8(node, "no-prop-u8", &val_u8); + KUNIT_EXPECT_NE(test, error, 0); + + error = fwnode_property_read_u8_array(node, "no-prop-u8", array_u8, 1); + KUNIT_EXPECT_NE(test, error, 0); + + error = fwnode_property_read_u16(node, "prop-u16", &val_u16); + KUNIT_EXPECT_EQ(test, error, 0); + KUNIT_EXPECT_EQ(test, (int)val_u16, 16); + + error = fwnode_property_read_u16_array(node, "prop-u16", array_u16, 1); + KUNIT_EXPECT_EQ(test, error, 0); + KUNIT_EXPECT_EQ(test, (int)array_u16[0], 16); + + error = fwnode_property_read_u16_array(node, "prop-u16", array_u16, 2); + KUNIT_EXPECT_NE(test, error, 0); + + error = fwnode_property_read_u16(node, "no-prop-u16", &val_u16); + KUNIT_EXPECT_NE(test, error, 0); + + error = fwnode_property_read_u16_array(node, "no-prop-u16", array_u16, 1); + KUNIT_EXPECT_NE(test, error, 0); + + error = fwnode_property_read_u32(node, "prop-u32", &val_u32); + KUNIT_EXPECT_EQ(test, error, 0); + KUNIT_EXPECT_EQ(test, (int)val_u32, 32); + + error = fwnode_property_read_u32_array(node, "prop-u32", array_u32, 1); + KUNIT_EXPECT_EQ(test, error, 0); + KUNIT_EXPECT_EQ(test, (int)array_u32[0], 32); + + error = fwnode_property_read_u32_array(node, "prop-u32", array_u32, 2); + KUNIT_EXPECT_NE(test, error, 0); + + error = fwnode_property_read_u32(node, "no-prop-u32", &val_u32); + KUNIT_EXPECT_NE(test, error, 0); + + error = fwnode_property_read_u32_array(node, "no-prop-u32", array_u32, 1); + KUNIT_EXPECT_NE(test, error, 0); + + error = fwnode_property_read_u64(node, "prop-u64", &val_u64); + KUNIT_EXPECT_EQ(test, error, 0); + KUNIT_EXPECT_EQ(test, (int)val_u64, 64); + + error = fwnode_property_read_u64_array(node, "prop-u64", array_u64, 1); + KUNIT_EXPECT_EQ(test, error, 0); + KUNIT_EXPECT_EQ(test, (int)array_u64[0], 64); + + error = fwnode_property_read_u64_array(node, "prop-u64", array_u64, 2); + KUNIT_EXPECT_NE(test, error, 0); + + error = fwnode_property_read_u64(node, "no-prop-u64", &val_u64); + KUNIT_EXPECT_NE(test, error, 0); + + error = fwnode_property_read_u64_array(node, "no-prop-u64", array_u64, 1); + KUNIT_EXPECT_NE(test, error, 0); + + fwnode_remove_software_node(node); +} + +static void pe_test_uint_arrays(struct kunit *test) +{ + u8 a_u8[16] = { 8, 9 }; + u16 a_u16[16] = { 16, 17 }; + u32 a_u32[16] = { 32, 33 }; + u64 a_u64[16] = { 64, 65 }; + const struct property_entry entries[] = { + PROPERTY_ENTRY_U8_ARRAY("prop-u8", a_u8), + PROPERTY_ENTRY_U16_ARRAY("prop-u16", a_u16), + PROPERTY_ENTRY_U32_ARRAY("prop-u32", a_u32), + PROPERTY_ENTRY_U64_ARRAY("prop-u64", a_u64), + { } + }; + + struct fwnode_handle *node; + u8 val_u8, array_u8[32]; + u16 val_u16, array_u16[32]; + u32 val_u32, array_u32[32]; + u64 val_u64, array_u64[32]; + int error; + + node = fwnode_create_software_node(entries, NULL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, node); + + error = fwnode_property_read_u8(node, "prop-u8", &val_u8); + KUNIT_EXPECT_EQ(test, error, 0); + KUNIT_EXPECT_EQ(test, (int)val_u8, 8); + + error = fwnode_property_read_u8_array(node, "prop-u8", array_u8, 1); + KUNIT_EXPECT_EQ(test, error, 0); + KUNIT_EXPECT_EQ(test, (int)array_u8[0], 8); + + error = fwnode_property_read_u8_array(node, "prop-u8", array_u8, 2); + KUNIT_EXPECT_EQ(test, error, 0); + KUNIT_EXPECT_EQ(test, (int)array_u8[0], 8); + KUNIT_EXPECT_EQ(test, (int)array_u8[1], 9); + + error = fwnode_property_read_u8_array(node, "prop-u8", array_u8, 17); + KUNIT_EXPECT_NE(test, error, 0); + + error = fwnode_property_read_u8(node, "no-prop-u8", &val_u8); + KUNIT_EXPECT_NE(test, error, 0); + + error = fwnode_property_read_u8_array(node, "no-prop-u8", array_u8, 1); + KUNIT_EXPECT_NE(test, error, 0); + + error = fwnode_property_read_u16(node, "prop-u16", &val_u16); + KUNIT_EXPECT_EQ(test, error, 0); + KUNIT_EXPECT_EQ(test, (int)val_u16, 16); + + error = fwnode_property_read_u16_array(node, "prop-u16", array_u16, 1); + KUNIT_EXPECT_EQ(test, error, 0); + KUNIT_EXPECT_EQ(test, (int)array_u16[0], 16); + + error = fwnode_property_read_u16_array(node, "prop-u16", array_u16, 2); + KUNIT_EXPECT_EQ(test, error, 0); + KUNIT_EXPECT_EQ(test, (int)array_u16[0], 16); + KUNIT_EXPECT_EQ(test, (int)array_u16[1], 17); + + error = fwnode_property_read_u16_array(node, "prop-u16", array_u16, 17); + KUNIT_EXPECT_NE(test, error, 0); + + error = fwnode_property_read_u16(node, "no-prop-u16", &val_u16); + KUNIT_EXPECT_NE(test, error, 0); + + error = fwnode_property_read_u16_array(node, "no-prop-u16", array_u16, 1); + KUNIT_EXPECT_NE(test, error, 0); + + error = fwnode_property_read_u32(node, "prop-u32", &val_u32); + KUNIT_EXPECT_EQ(test, error, 0); + KUNIT_EXPECT_EQ(test, (int)val_u32, 32); + + error = fwnode_property_read_u32_array(node, "prop-u32", array_u32, 1); + KUNIT_EXPECT_EQ(test, error, 0); + KUNIT_EXPECT_EQ(test, (int)array_u32[0], 32); + + error = fwnode_property_read_u32_array(node, "prop-u32", array_u32, 2); + KUNIT_EXPECT_EQ(test, error, 0); + KUNIT_EXPECT_EQ(test, (int)array_u32[0], 32); + KUNIT_EXPECT_EQ(test, (int)array_u32[1], 33); + + error = fwnode_property_read_u32_array(node, "prop-u32", array_u32, 17); + KUNIT_EXPECT_NE(test, error, 0); + + error = fwnode_property_read_u32(node, "no-prop-u32", &val_u32); + KUNIT_EXPECT_NE(test, error, 0); + + error = fwnode_property_read_u32_array(node, "no-prop-u32", array_u32, 1); + KUNIT_EXPECT_NE(test, error, 0); + + error = fwnode_property_read_u64(node, "prop-u64", &val_u64); + KUNIT_EXPECT_EQ(test, error, 0); + KUNIT_EXPECT_EQ(test, (int)val_u64, 64); + + error = fwnode_property_read_u64_array(node, "prop-u64", array_u64, 1); + KUNIT_EXPECT_EQ(test, error, 0); + KUNIT_EXPECT_EQ(test, (int)array_u64[0], 64); + + error = fwnode_property_read_u64_array(node, "prop-u64", array_u64, 2); + KUNIT_EXPECT_EQ(test, error, 0); + KUNIT_EXPECT_EQ(test, (int)array_u64[0], 64); + KUNIT_EXPECT_EQ(test, (int)array_u64[1], 65); + + error = fwnode_property_read_u64_array(node, "prop-u64", array_u64, 17); + KUNIT_EXPECT_NE(test, error, 0); + + error = fwnode_property_read_u64(node, "no-prop-u64", &val_u64); + KUNIT_EXPECT_NE(test, error, 0); + + error = fwnode_property_read_u64_array(node, "no-prop-u64", array_u64, 1); + KUNIT_EXPECT_NE(test, error, 0); + + fwnode_remove_software_node(node); +} + +static void pe_test_strings(struct kunit *test) +{ + const char *strings[] = { + "string-a", + "string-b", + }; + + const struct property_entry entries[] = { + PROPERTY_ENTRY_STRING("str", "single"), + PROPERTY_ENTRY_STRING("empty", ""), + PROPERTY_ENTRY_STRING_ARRAY("strs", strings), + { } + }; + + struct fwnode_handle *node; + const char *str; + const char *strs[10]; + int error; + + node = fwnode_create_software_node(entries, NULL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, node); + + error = fwnode_property_read_string(node, "str", &str); + KUNIT_EXPECT_EQ(test, error, 0); + KUNIT_EXPECT_STREQ(test, str, "single"); + + error = fwnode_property_read_string_array(node, "str", strs, 1); + KUNIT_EXPECT_EQ(test, error, 1); + KUNIT_EXPECT_STREQ(test, strs[0], "single"); + + /* asking for more data returns what we have */ + error = fwnode_property_read_string_array(node, "str", strs, 2); + KUNIT_EXPECT_EQ(test, error, 1); + KUNIT_EXPECT_STREQ(test, strs[0], "single"); + + error = fwnode_property_read_string(node, "no-str", &str); + KUNIT_EXPECT_NE(test, error, 0); + + error = fwnode_property_read_string_array(node, "no-str", strs, 1); + KUNIT_EXPECT_LT(test, error, 0); + + error = fwnode_property_read_string(node, "empty", &str); + KUNIT_EXPECT_EQ(test, error, 0); + KUNIT_EXPECT_STREQ(test, str, ""); + + error = fwnode_property_read_string_array(node, "strs", strs, 3); + KUNIT_EXPECT_EQ(test, error, 2); + KUNIT_EXPECT_STREQ(test, strs[0], "string-a"); + KUNIT_EXPECT_STREQ(test, strs[1], "string-b"); + + error = fwnode_property_read_string_array(node, "strs", strs, 1); + KUNIT_EXPECT_EQ(test, error, 1); + KUNIT_EXPECT_STREQ(test, strs[0], "string-a"); + + /* NULL argument -> returns size */ + error = fwnode_property_read_string_array(node, "strs", NULL, 0); + KUNIT_EXPECT_EQ(test, error, 2); + + /* accessing array as single value */ + error = fwnode_property_read_string(node, "strs", &str); + KUNIT_EXPECT_EQ(test, error, 0); + KUNIT_EXPECT_STREQ(test, str, "string-a"); + + fwnode_remove_software_node(node); +} + +static void pe_test_bool(struct kunit *test) +{ + const struct property_entry entries[] = { + PROPERTY_ENTRY_BOOL("prop"), + { } + }; + + struct fwnode_handle *node; + + node = fwnode_create_software_node(entries, NULL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, node); + + KUNIT_EXPECT_TRUE(test, fwnode_property_read_bool(node, "prop")); + KUNIT_EXPECT_FALSE(test, fwnode_property_read_bool(node, "not-prop")); + + fwnode_remove_software_node(node); +} + +/* Verifies that small U8 array is stored inline when property is copied */ +static void pe_test_move_inline_u8(struct kunit *test) +{ + u8 u8_array_small[8] = { 1, 2, 3, 4 }; + u8 u8_array_big[128] = { 5, 6, 7, 8 }; + struct property_entry entries[] = { + PROPERTY_ENTRY_U8_ARRAY("small", u8_array_small), + PROPERTY_ENTRY_U8_ARRAY("big", u8_array_big), + { } + }; + struct property_entry *copy; + const u8 *data_ptr; + + copy = property_entries_dup(entries); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, copy); + + KUNIT_EXPECT_TRUE(test, copy[0].is_inline); + data_ptr = (u8 *)©[0].value; + KUNIT_EXPECT_EQ(test, (int)data_ptr[0], 1); + KUNIT_EXPECT_EQ(test, (int)data_ptr[1], 2); + + KUNIT_EXPECT_FALSE(test, copy[1].is_inline); + data_ptr = copy[1].pointer; + KUNIT_EXPECT_EQ(test, (int)data_ptr[0], 5); + KUNIT_EXPECT_EQ(test, (int)data_ptr[1], 6); + + property_entries_free(copy); +} + +/* Verifies that single string array is stored inline when property is copied */ +static void pe_test_move_inline_str(struct kunit *test) +{ + char *str_array_small[] = { "a" }; + char *str_array_big[] = { "b", "c", "d", "e" }; + char *str_array_small_empty[] = { "" }; + struct property_entry entries[] = { + PROPERTY_ENTRY_STRING_ARRAY("small", str_array_small), + PROPERTY_ENTRY_STRING_ARRAY("big", str_array_big), + PROPERTY_ENTRY_STRING_ARRAY("small-empty", str_array_small_empty), + { } + }; + struct property_entry *copy; + const char * const *data_ptr; + + copy = property_entries_dup(entries); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, copy); + + KUNIT_EXPECT_TRUE(test, copy[0].is_inline); + KUNIT_EXPECT_STREQ(test, copy[0].value.str[0], "a"); + + KUNIT_EXPECT_FALSE(test, copy[1].is_inline); + data_ptr = copy[1].pointer; + KUNIT_EXPECT_STREQ(test, data_ptr[0], "b"); + KUNIT_EXPECT_STREQ(test, data_ptr[1], "c"); + + KUNIT_EXPECT_TRUE(test, copy[2].is_inline); + KUNIT_EXPECT_STREQ(test, copy[2].value.str[0], ""); + + property_entries_free(copy); +} + +/* Handling of reference properties */ +static void pe_test_reference(struct kunit *test) +{ + const struct software_node nodes[] = { + { .name = "1", }, + { .name = "2", }, + }; + + const struct software_node_ref_args refs[] = { + { + .node = &nodes[0], + .nargs = 0, + }, + { + .node = &nodes[1], + .nargs = 2, + .args = { 3, 4 }, + }, + }; + + const struct property_entry entries[] = { + PROPERTY_ENTRY_REF("ref-1", &nodes[0]), + PROPERTY_ENTRY_REF("ref-2", &nodes[1], 1, 2), + PROPERTY_ENTRY_REF_ARRAY("ref-3", refs), + { } + }; + + struct fwnode_handle *node; + struct fwnode_reference_args ref; + int error; + + error = software_node_register_nodes(nodes); + KUNIT_ASSERT_EQ(test, error, 0); + + node = fwnode_create_software_node(entries, NULL); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, node); + + error = fwnode_property_get_reference_args(node, "ref-1", NULL, + 0, 0, &ref); + KUNIT_ASSERT_EQ(test, error, 0); + KUNIT_EXPECT_PTR_EQ(test, to_software_node(ref.fwnode), &nodes[0]); + KUNIT_EXPECT_EQ(test, ref.nargs, 0U); + + /* wrong index */ + error = fwnode_property_get_reference_args(node, "ref-1", NULL, + 0, 1, &ref); + KUNIT_EXPECT_NE(test, error, 0); + + error = fwnode_property_get_reference_args(node, "ref-2", NULL, + 1, 0, &ref); + KUNIT_ASSERT_EQ(test, error, 0); + KUNIT_EXPECT_PTR_EQ(test, to_software_node(ref.fwnode), &nodes[1]); + KUNIT_EXPECT_EQ(test, ref.nargs, 1U); + KUNIT_EXPECT_EQ(test, ref.args[0], 1LLU); + + /* asking for more args, padded with zero data */ + error = fwnode_property_get_reference_args(node, "ref-2", NULL, + 3, 0, &ref); + KUNIT_ASSERT_EQ(test, error, 0); + KUNIT_EXPECT_PTR_EQ(test, to_software_node(ref.fwnode), &nodes[1]); + KUNIT_EXPECT_EQ(test, ref.nargs, 3U); + KUNIT_EXPECT_EQ(test, ref.args[0], 1LLU); + KUNIT_EXPECT_EQ(test, ref.args[1], 2LLU); + KUNIT_EXPECT_EQ(test, ref.args[2], 0LLU); + + /* wrong index */ + error = fwnode_property_get_reference_args(node, "ref-2", NULL, + 2, 1, &ref); + KUNIT_EXPECT_NE(test, error, 0); + + /* array of references */ + error = fwnode_property_get_reference_args(node, "ref-3", NULL, + 0, 0, &ref); + KUNIT_ASSERT_EQ(test, error, 0); + KUNIT_EXPECT_PTR_EQ(test, to_software_node(ref.fwnode), &nodes[0]); + KUNIT_EXPECT_EQ(test, ref.nargs, 0U); + + /* second reference in the array */ + error = fwnode_property_get_reference_args(node, "ref-3", NULL, + 2, 1, &ref); + KUNIT_ASSERT_EQ(test, error, 0); + KUNIT_EXPECT_PTR_EQ(test, to_software_node(ref.fwnode), &nodes[1]); + KUNIT_EXPECT_EQ(test, ref.nargs, 2U); + KUNIT_EXPECT_EQ(test, ref.args[0], 3LLU); + KUNIT_EXPECT_EQ(test, ref.args[1], 4LLU); + + /* wrong index */ + error = fwnode_property_get_reference_args(node, "ref-1", NULL, + 0, 2, &ref); + KUNIT_EXPECT_NE(test, error, 0); + + fwnode_remove_software_node(node); + software_node_unregister_nodes(nodes); +} + +static struct kunit_case property_entry_test_cases[] = { + KUNIT_CASE(pe_test_uints), + KUNIT_CASE(pe_test_uint_arrays), + KUNIT_CASE(pe_test_strings), + KUNIT_CASE(pe_test_bool), + KUNIT_CASE(pe_test_move_inline_u8), + KUNIT_CASE(pe_test_move_inline_str), + KUNIT_CASE(pe_test_reference), + { } +}; + +static struct kunit_suite property_entry_test_suite = { + .name = "property-entry", + .test_cases = property_entry_test_cases, +}; + +kunit_test_suite(property_entry_test_suite); -- 2.24.0.rc1.363.gb1bccd3e3d-goog ^ permalink raw reply related [flat|nested] 28+ messages in thread
end of thread, other threads:[~2020-11-10 12:47 UTC | newest] Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-11-08 4:22 [PATCH v8 0/6] software node: add support for reference properties Dmitry Torokhov 2019-11-08 4:22 ` [PATCH v8 1/6] software node: rename is_array to is_inline Dmitry Torokhov 2019-11-08 9:49 ` Rafael J. Wysocki 2019-11-13 6:52 ` Bjørn Mork 2019-11-13 8:08 ` Dmitry Torokhov [not found] ` <CGME20191212111237eucas1p1a278d2d5d2437e3219896367e82604cc@eucas1p1.samsung.com> 2019-12-12 11:12 ` Marek Szyprowski 2019-12-12 11:28 ` Andy Shevchenko 2019-12-12 16:41 ` Rafael J. Wysocki 2019-12-13 6:47 ` Marek Szyprowski 2019-12-13 8:37 ` Rafael J. Wysocki 2019-12-13 1:24 ` Dmitry Torokhov 2019-12-13 6:44 ` Marek Szyprowski 2019-11-08 4:22 ` [PATCH v8 2/6] software node: allow embedding of small arrays into property_entry Dmitry Torokhov 2019-11-08 4:22 ` [PATCH v8 3/6] software node: implement reference properties Dmitry Torokhov [not found] ` <CGME20201109170241eucas1p14c2156334d8c6ef15d52664fa4776f41@eucas1p1.samsung.com> 2020-11-09 17:02 ` Lukasz Stelmach 2020-11-09 17:24 ` Andy Shevchenko [not found] ` <CGME20201109181851eucas1p241de8938e399c0b603c764593b057c41@eucas1p2.samsung.com> 2020-11-09 18:18 ` Lukasz Stelmach 2020-11-09 18:53 ` Dmitry Torokhov 2020-11-09 19:05 ` Andy Shevchenko 2020-11-10 12:39 ` Heikki Krogerus 2020-11-10 12:46 ` Rafael J. Wysocki [not found] ` <CGME20201109195504eucas1p19d493c947d8752e39c26202cf0978fc0@eucas1p1.samsung.com> 2020-11-09 19:54 ` Lukasz Stelmach 2020-11-09 19:02 ` Andy Shevchenko [not found] ` <CGME20201109194725eucas1p2cc9357486879a14b2ad2f6ef968ff4b2@eucas1p2.samsung.com> 2020-11-09 19:47 ` Lukasz Stelmach 2020-11-09 21:20 ` Andy Shevchenko 2019-11-08 4:22 ` [PATCH v8 4/6] platform/x86: intel_cht_int33fe: use inline " Dmitry Torokhov 2019-11-08 4:22 ` [PATCH v8 5/6] software node: remove separate handling of references Dmitry Torokhov 2019-11-08 4:22 ` [PATCH v8 6/6] software node: add basic tests for property entries Dmitry Torokhov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).