| Message ID | 87fryoud8t.wl-kuninori.morimoto.gx@renesas.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [01/13] of: property: add port base loop | expand |
Hi Kuninori, kernel test robot noticed the following build warnings: [auto build test WARNING on broonie-sound/for-next] [also build test WARNING on drm-misc/drm-misc-next linus/master v6.8-rc1 next-20240125] [cannot apply to robh/for-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Kuninori-Morimoto/of-property-add-port-base-loop/20240123-081427 base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next patch link: https://lore.kernel.org/r/87fryoud8t.wl-kuninori.morimoto.gx%40renesas.com patch subject: [PATCH 03/13] of: property: add of_graph_get_next_endpoint_raw() config: i386-buildonly-randconfig-002-20240125 (https://download.01.org/0day-ci/archive/20240126/202401260024.txulvo50-lkp@intel.com/config) compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240126/202401260024.txulvo50-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202401260024.txulvo50-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/of/property.c:681: warning: Function parameter or struct member 'port' not described in 'of_graph_get_next_endpoint_raw' >> drivers/of/property.c:681: warning: Excess function parameter 'parent' description in 'of_graph_get_next_endpoint_raw' vim +681 drivers/of/property.c 670 671 /** 672 * of_graph_get_next_endpoint_raw() - get next endpoint node 673 * @parent: pointer to the target port node 674 * @endpoint: current endpoint node, or NULL to get first 675 * 676 * Return: An 'endpoint' node pointer with refcount incremented. Refcount 677 * of the passed @prev node is decremented. 678 */ 679 struct device_node *of_graph_get_next_endpoint_raw(const struct device_node *port, 680 struct device_node *endpoint) > 681 { 682 if (!port) 683 return NULL; 684 685 do { 686 endpoint = of_get_next_child(port, endpoint); 687 if (!endpoint) 688 break; 689 } while (!of_node_name_eq(endpoint, "endpoint")); 690 691 return endpoint; 692 } 693 EXPORT_SYMBOL(of_graph_get_next_endpoint_raw); 694
diff --git a/drivers/of/property.c b/drivers/of/property.c index 14ffd199c9b1..e2d179cf7d26 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -667,6 +667,30 @@ struct device_node *of_graph_get_next_port(const struct device_node *parent, } EXPORT_SYMBOL(of_graph_get_next_port); +/** + * of_graph_get_next_endpoint_raw() - get next endpoint node + * @parent: pointer to the target port node + * @endpoint: current endpoint node, or NULL to get first + * + * Return: An 'endpoint' node pointer with refcount incremented. Refcount + * of the passed @prev node is decremented. + */ +struct device_node *of_graph_get_next_endpoint_raw(const struct device_node *port, + struct device_node *endpoint) +{ + if (!port) + return NULL; + + do { + endpoint = of_get_next_child(port, endpoint); + if (!endpoint) + break; + } while (!of_node_name_eq(endpoint, "endpoint")); + + return endpoint; +} +EXPORT_SYMBOL(of_graph_get_next_endpoint_raw); + /** * of_graph_get_next_endpoint() - get next endpoint node * @parent: pointer to the parent device node @@ -708,7 +732,7 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent, * getting the next child. If the previous endpoint is NULL this * will return the first child. */ - endpoint = of_get_next_child(port, prev); + endpoint = of_graph_get_next_endpoint_raw(port, prev); if (endpoint) { of_node_put(port); return endpoint; diff --git a/include/linux/of_graph.h b/include/linux/of_graph.h index fff598640e93..427905a6e8c3 100644 --- a/include/linux/of_graph.h +++ b/include/linux/of_graph.h @@ -57,6 +57,8 @@ int of_graph_get_port_count(const struct device_node *np); struct device_node *of_graph_get_port_by_id(struct device_node *node, u32 id); struct device_node *of_graph_get_next_endpoint(const struct device_node *parent, struct device_node *previous); +struct device_node *of_graph_get_next_endpoint_raw(const struct device_node *port, + struct device_node *prev); struct device_node *of_graph_get_next_port(const struct device_node *parent, struct device_node *previous); struct device_node *of_graph_get_endpoint_by_regs(
We already have of_graph_get_next_endpoint(), but it is not intuitive to use. (X) node { (Y) ports { port@0 { endpoint { remote-endpoint = ...; };}; (A1) port@1 { endpoint { remote-endpoint = ...; }; (A2) endpoint { remote-endpoint = ...; };}; (B) port@2 { endpoint { remote-endpoint = ...; };}; }; }; For example, if I want to handle port@1's 2 endpoints (= A1, A2), I want to use like below A1 = of_graph_get_next_endpoint(port1, NULL); A2 = of_graph_get_next_endpoint(port1, A1); But 1st one will be error, because of_graph_get_next_endpoint() requested "parent" means "node" (X) or "ports" (Y), not "port". Below are OK of_graph_get_next_endpoint(node, NULL); // node/ports/port@0/endpoint of_graph_get_next_endpoint(ports, NULL); // node/ports/port@0/endpoint In other words, we can't handle A1/A2 directly via of_graph_get_next_endpoint() so far. There is another non intuitive behavior on of_graph_get_next_endpoint(). In case of if I could get A1 pointer for some way, and if I want to handle port@1 things, I would like use it like below /* * "endpoint" is now A1, and handle port1 things here, * but we don't know how many endpoints port1 has. * * Because "endpoint" is non NULL, we can use port1 * as of_graph_get_next_endpoint(port1, xxx) */ do { /* do something for port1 specific things here */ } while (endpoint = of_graph_get_next_endpoint(port1, endpoint)) But it also not worked as I expected. I expect it will be A1 -> A2 -> NULL, but it will be A1 -> A2 -> B, because of_graph_get_next_endpoint() will fetch endpoint beyond the port. It is not useful on generic driver like Generic Sound Card. It uses of_get_next_child() instead for now, but it is not intuitive, and not check node name (= "endpoint"). To handle endpoint more intuitive, create of_graph_get_next_endpoint_raw() of_graph_get_next_endpoint_raw(port1, NULL); // A1 of_graph_get_next_endpoint_raw(port1, A1); // A2 of_graph_get_next_endpoint_raw(port1, A2); // NULL Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> --- drivers/of/property.c | 26 +++++++++++++++++++++++++- include/linux/of_graph.h | 2 ++ 2 files changed, 27 insertions(+), 1 deletion(-)