On Sat, Sep 07, 2019 at 07:08:19PM +0300, Andy Shevchenko wrote: > On Fri, Sep 06, 2019 at 03:26:09PM -0700, 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), > > { } > > }; > > > > Thanks for an update, my comments below. > > > + } else if (src->type == DEV_PROP_REF) { > > + /* All reference properties must be arrays */ > > + return -EINVAL; > > Hmm... What about to duplicate pointer under value union and use is_array to > distinguish which one to use? Because...
Then we have to special-case copying this entry, similar to the pains we are going with the strings. > > > > @@ -240,6 +254,7 @@ struct property_entry { > > const u32 *u32_data; > > const u64 *u64_data; > > const char * const *str; > > + const struct software_node_ref_args *ref; > > } pointer; > > > +#define PROPERTY_ENTRY_REF(_name_, _ref_, ...) > > \ > > +(struct property_entry) { \ > > + .name = _name_, \ > > > + .length = sizeof(struct software_node_ref_args), \ > > Is it correct? Yes, we length is element size * number of elements. > > > + .type = DEV_PROP_REF, \ > > > + .is_array = true, \ > > I really don't like this "cheating". This is not cheating. Any single value can be represented as an array of one element. Actually, the only reason we have this "is_array" business is because for scalar values and short strings it is much cheaper to store single value in-line instead of out of line + pointer, especially on 64 bit arches. If you want we can change is_array into is_inline. Thanks. -- Dmitry