| Message ID | 20240901040658.157425-11-swboyd@chromium.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | platform/chrome: Add DT USB/DP muxing/topology support | expand |
On Sat, Aug 31, 2024 at 09:06:48PM -0700, Stephen Boyd wrote: > The usage of this match function is hard to understand at a glance. > Document the arguments and the return value so it is clear how to > implement the function. Thank you for the patch! ... I believe we still use "device property:" in the subject for this header file changes. $ git log --oneline --no-merges -- include/linux/property.h ... > +/** > + * devcon_match_fn_t - device connection match function > + * @fwnode: Remote connection's device node > + * @con_id: Identifier for the connection > + * @data: Match function caller specific data > + * > + * Implement a callback with this function signature to search a fwnode's > + * connections for a match with a function like device_connection_find_match(). > + * This function will be called possibly multiple times, once for each > + * connection. The match function should inspect the @fwnode to look for a > + * match. The @con_id and @data provided are the same as the @con_id and @data > + * arguments passed to the functions that take a devcon_match_fn_t argument. > + * Note: This function can be called multiple times. As noted in the next patch, this would be nice to elaborate (at least to me this sounds like declaration of idempotency which is unlikely what is meant, or am I mistaken?). > + * Return: Pointer to match or NULL if no match found. > + */ > +typedef void *(*devcon_match_fn_t)(const struct fwnode_handle *fwnode, > + const char *con_id, void *data);
Quoting Andy Shevchenko (2024-09-02 04:17:12) > On Sat, Aug 31, 2024 at 09:06:48PM -0700, Stephen Boyd wrote: > > The usage of this match function is hard to understand at a glance. > > Document the arguments and the return value so it is clear how to > > implement the function. > > Thank you for the patch! > > ... > > I believe we still use "device property:" in the subject for this header file changes. > $ git log --oneline --no-merges -- include/linux/property.h > Ok. > > > +/** > > + * devcon_match_fn_t - device connection match function > > + * @fwnode: Remote connection's device node > > + * @con_id: Identifier for the connection > > + * @data: Match function caller specific data > > + * > > + * Implement a callback with this function signature to search a fwnode's > > + * connections for a match with a function like device_connection_find_match(). > > + * This function will be called possibly multiple times, once for each > > + * connection. The match function should inspect the @fwnode to look for a > > + * match. The @con_id and @data provided are the same as the @con_id and @data > > + * arguments passed to the functions that take a devcon_match_fn_t argument. > > > + * Note: This function can be called multiple times. > > As noted in the next patch, this would be nice to elaborate (at least to me > this sounds like declaration of idempotency which is unlikely what is > meant, or am I mistaken?). I based this on something that I've already forgotten! :) It's saying that the function you implement shouldn't have side-effects because it will be called many times. I actually wrote above that it will be called "possibly multiple times, once for each connection". Let me try to remove "multiple times".
diff --git a/include/linux/property.h b/include/linux/property.h index 61fc20e5f81f..797b1eeda7d2 100644 --- a/include/linux/property.h +++ b/include/linux/property.h @@ -507,8 +507,25 @@ unsigned int fwnode_graph_get_endpoint_count(const struct fwnode_handle *fwnode, int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode, struct fwnode_endpoint *endpoint); -typedef void *(*devcon_match_fn_t)(const struct fwnode_handle *fwnode, const char *id, - void *data); +/** + * devcon_match_fn_t - device connection match function + * @fwnode: Remote connection's device node + * @con_id: Identifier for the connection + * @data: Match function caller specific data + * + * Implement a callback with this function signature to search a fwnode's + * connections for a match with a function like device_connection_find_match(). + * This function will be called possibly multiple times, once for each + * connection. The match function should inspect the @fwnode to look for a + * match. The @con_id and @data provided are the same as the @con_id and @data + * arguments passed to the functions that take a devcon_match_fn_t argument. + * + * Note: This function can be called multiple times. + * + * Return: Pointer to match or NULL if no match found. + */ +typedef void *(*devcon_match_fn_t)(const struct fwnode_handle *fwnode, + const char *con_id, void *data); void *fwnode_connection_find_match(const struct fwnode_handle *fwnode, const char *con_id, void *data,
The usage of this match function is hard to understand at a glance. Document the arguments and the return value so it is clear how to implement the function. Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Signed-off-by: Stephen Boyd <swboyd@chromium.org> --- include/linux/property.h | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-)