On 4/8/25 04:39, Andy Shevchenko wrote:
> On Mon, Apr 07, 2025 at 06:37:14PM -0400, Sean Anderson wrote:
>> Add a fwnode variant of of_parse_phandle_with_optional_args to allow
>> nargs_prop to be absent from the referenced node. This improves
>> compatibility for references where the devicetree might not always have
>> nargs_prop.
>
> ...
>
>> +/**
>> + * fwnode_property_get_reference_optional_args() - Find a reference with optional arguments
>> + * @fwnode: Firmware node where to look for the reference
>> + * @prop: The name of the property
>> + * @nargs_prop: The name of the property telling the number of
>
> Use space instead of TAB as it's already too long to make it aligned with the
> rest.
>
>> + * arguments in the referred node.
>> + * @index: Index of the reference, from zero onwards.
>> + * @args: Result structure with reference and integer arguments.
>> + * May be NULL.
>> + *
>> + * Obtain a reference based on a named property in an fwnode, with
>> + * integer arguments. If @nargs_prop is absent from the referenced node, then
>> + * number of arguments is be assumed to be 0.
>> + *
>> + * The caller is responsible for calling fwnode_handle_put() on the returned
>> + * @args->fwnode pointer.
>> + *
>> + * Return: %0 on success
>> + * %-ENOENT when the index is out of bounds, the index has an empty
>> + * reference or the property was not found
>> + * %-EINVAL on parse error
>> + */
>> +int fwnode_property_get_reference_optional_args(const struct fwnode_handle *fwnode,
>> + const char *prop,
>> + const char *nargs_prop,
>> + unsigned int index,
>> + struct fwnode_reference_args *args)
>> +{
>> + int ret;
>
>> + if (IS_ERR_OR_NULL(fwnode))
>> + return -ENOENT;
>
> This is incorrect most likely, see below.
>
>> + ret = fwnode_call_int_op(fwnode, get_reference_args, prop, nargs_prop,
>> + 0, index, args);
>> + if (ret == 0)
>> + return ret;
>> +
>> + if (IS_ERR_OR_NULL(fwnode->secondary))
>> + return ret;
>
> Here no such error code shadowing, and TBH I do not like the shadowing without
> real need.
I don't understand the objection. First, this logic is identical to
fwnode_property_get_reference_args. Second, the process seems clear to
me:
- If we have a primary fwnode, try it otherwise return -ENOENT
- If we have a secondary fwnode and the first failed, try it otherwise
return the original error code
The purpose of a secondary fwnode is to allow supplying missing
properties absent from the primary fwnode. Which part of the above do
you dislike?
--Sean
