| Message ID | 20210329151207.36619-5-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/6] software node: Free resources explicitly when swnode_register() fails | expand |
Hi Andy On 29/03/2021 16:12, Andy Shevchenko wrote: > This is useful to assign software node reference with arguments > in a common way. Moreover, we have already couple of users that > may be converted. And by the fact, one of them is moved right here > to use the helper. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > v2: no changes > drivers/base/test/property-entry-test.c | 11 ++--------- > include/linux/property.h | 13 ++++++++----- > 2 files changed, 10 insertions(+), 14 deletions(-) > > diff --git a/drivers/base/test/property-entry-test.c b/drivers/base/test/property-entry-test.c > index abe03315180f..c2e455d46ffd 100644 > --- a/drivers/base/test/property-entry-test.c > +++ b/drivers/base/test/property-entry-test.c > @@ -370,15 +370,8 @@ static void pe_test_reference(struct kunit *test) > }; > > static const struct software_node_ref_args refs[] = { > - { > - .node = &nodes[0], > - .nargs = 0, > - }, > - { > - .node = &nodes[1], > - .nargs = 2, > - .args = { 3, 4 }, > - }, > + SOFTWARE_NODE_REFERENCE(&nodes[0]), > + SOFTWARE_NODE_REFERENCE(&nodes[1], 3, 4), > }; > > const struct property_entry entries[] = { > diff --git a/include/linux/property.h b/include/linux/property.h > index dd4687b56239..0d876316e61d 100644 > --- a/include/linux/property.h > +++ b/include/linux/property.h > @@ -254,6 +254,13 @@ struct software_node_ref_args { > u64 args[NR_FWNODE_REFERENCE_ARGS]; > }; > > +#define SOFTWARE_NODE_REFERENCE(_ref_, ...) \ > +(const struct software_node_ref_args) { \ > + .node = _ref_, \ > + .nargs = ARRAY_SIZE(((u64[]){ 0, ##__VA_ARGS__ })) - 1, \ > + .args = { __VA_ARGS__ }, \ > +} > + > /** > * struct property_entry - "Built-in" device property representation. > * @name: Name of the property. > @@ -362,11 +369,7 @@ 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__ }, \ > - } }, \ > + { .pointer = &SOFTWARE_NODE_REFERENCE(_ref_, ##__VA_ARGS__), }, \ > } What are the .args intended to be used for? I actually had it in mind to replace this with a simple pointer to a struct software_node, because I can't see any users of them and the fact that it's actually storing a pointer to a new variable is something that confused me for a good long time when I wrote the cio2-bridge (though that's mostly due to my relative inexperience of course, but still) > > struct property_entry *
On Mon, Mar 29, 2021 at 11:45:29PM +0100, Daniel Scally wrote: > On 29/03/2021 16:12, Andy Shevchenko wrote: > > This is useful to assign software node reference with arguments > > in a common way. Moreover, we have already couple of users that > > may be converted. And by the fact, one of them is moved right here > > to use the helper. ... > > + SOFTWARE_NODE_REFERENCE(&nodes[0]), > > + SOFTWARE_NODE_REFERENCE(&nodes[1], 3, 4), ... > > +#define SOFTWARE_NODE_REFERENCE(_ref_, ...) \ > > +(const struct software_node_ref_args) { \ > > + .node = _ref_, \ > > + .nargs = ARRAY_SIZE(((u64[]){ 0, ##__VA_ARGS__ })) - 1, \ > > + .args = { __VA_ARGS__ }, \ > > +} ... > > + { .pointer = &SOFTWARE_NODE_REFERENCE(_ref_, ##__VA_ARGS__), }, \ > > What are the .args intended to be used for? I actually had it in mind to > replace this with a simple pointer to a struct software_node, because I > can't see any users of them and the fact that it's actually storing a > pointer to a new variable is something that confused me for a good long > time when I wrote the cio2-bridge (though that's mostly due to my > relative inexperience of course, but still) It's to be in align with DT phandle references that can take arguments. While for now, indeed, we have no users of this, it might be changed in the future (I hadn't checked DesignWare DMA where I would like to transform the code to use device properties eventually and there it might be the case).
Hi Andy On 30/03/2021 10:26, Andy Shevchenko wrote: >>> + { .pointer = &SOFTWARE_NODE_REFERENCE(_ref_, ##__VA_ARGS__), }, \ >> What are the .args intended to be used for? I actually had it in mind to >> replace this with a simple pointer to a struct software_node, because I >> can't see any users of them and the fact that it's actually storing a >> pointer to a new variable is something that confused me for a good long >> time when I wrote the cio2-bridge (though that's mostly due to my >> relative inexperience of course, but still) > It's to be in align with DT phandle references that can take arguments. While > for now, indeed, we have no users of this, it might be changed in the future > (I hadn't checked DesignWare DMA where I would like to transform the code to > use device properties eventually and there it might be the case). Ah yeah I see - haven't come across phandles before but having looked them up now I see what this is meant to emulate. Consistency is good; in that case, for this and 6/6: Reviewed-by: Daniel Scally <djrscally@gmail.com> and Tested-by: Daniel Scally <djrscally@gmail.com> >
diff --git a/drivers/base/test/property-entry-test.c b/drivers/base/test/property-entry-test.c index abe03315180f..c2e455d46ffd 100644 --- a/drivers/base/test/property-entry-test.c +++ b/drivers/base/test/property-entry-test.c @@ -370,15 +370,8 @@ static void pe_test_reference(struct kunit *test) }; static const struct software_node_ref_args refs[] = { - { - .node = &nodes[0], - .nargs = 0, - }, - { - .node = &nodes[1], - .nargs = 2, - .args = { 3, 4 }, - }, + SOFTWARE_NODE_REFERENCE(&nodes[0]), + SOFTWARE_NODE_REFERENCE(&nodes[1], 3, 4), }; const struct property_entry entries[] = { diff --git a/include/linux/property.h b/include/linux/property.h index dd4687b56239..0d876316e61d 100644 --- a/include/linux/property.h +++ b/include/linux/property.h @@ -254,6 +254,13 @@ struct software_node_ref_args { u64 args[NR_FWNODE_REFERENCE_ARGS]; }; +#define SOFTWARE_NODE_REFERENCE(_ref_, ...) \ +(const struct software_node_ref_args) { \ + .node = _ref_, \ + .nargs = ARRAY_SIZE(((u64[]){ 0, ##__VA_ARGS__ })) - 1, \ + .args = { __VA_ARGS__ }, \ +} + /** * struct property_entry - "Built-in" device property representation. * @name: Name of the property. @@ -362,11 +369,7 @@ 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__ }, \ - } }, \ + { .pointer = &SOFTWARE_NODE_REFERENCE(_ref_, ##__VA_ARGS__), }, \ } struct property_entry *
This is useful to assign software node reference with arguments in a common way. Moreover, we have already couple of users that may be converted. And by the fact, one of them is moved right here to use the helper. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- v2: no changes drivers/base/test/property-entry-test.c | 11 ++--------- include/linux/property.h | 13 ++++++++----- 2 files changed, 10 insertions(+), 14 deletions(-)