From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753165AbdBFOE1 (ORCPT ); Mon, 6 Feb 2017 09:04:27 -0500 Received: from metis.ext.4.pengutronix.de ([92.198.50.35]:42269 "EHLO metis.ext.4.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752912AbdBFOEV (ORCPT ); Mon, 6 Feb 2017 09:04:21 -0500 Message-ID: <1486389834.3005.55.camel@pengutronix.de> Subject: Re: [PATCH 1/5] of: introduce of_graph_get_remote_node From: Philipp Zabel To: Rob Herring Cc: David Airlie , Daniel Vetter , Sean Paul , dri-devel , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Frank Rowand , Boris Brezillon , Archit Taneja , Jingoo Han , Inki Dae , Joonyoung Shim , Seung-Woo Kim , Kyungmin Park , Kukjin Kim , Krzysztof Kozlowski , Javier Martinez Canillas , Stefan Agner , Alison Wang , Xinliang Liu , Rongrong Zou , Xinwei Kong , Chen Feng , CK Hu , Matthias Brugger , Marek Vasut , Mark Yao , Heiko Stuebner , Maxime Ripard , Chen-Yu Tsai , Liviu Dudau , Mali DP Maintainers , Neil Armstrong , Carlo Caione , Kevin Hilman , Rob Clark , Jyri Sarha , Tomi Valkeinen , Eric Anholt , Russell King Date: Mon, 06 Feb 2017 15:03:54 +0100 In-Reply-To: References: <20170204033635.10250-1-robh@kernel.org> <20170204033635.10250-2-robh@kernel.org> <1486377167.3005.30.camel@pengutronix.de> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.12.9-1+b1 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-SA-Exim-Connect-IP: 2001:67c:670:100:3ad5:47ff:feaf:1a17 X-SA-Exim-Mail-From: p.zabel@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 2017-02-06 at 07:54 -0600, Rob Herring wrote: > On Mon, Feb 6, 2017 at 4:32 AM, Philipp Zabel wrote: > > Hi Rob, > > > > thanks for this clean-up series! I was not aware how far the duplication > > has spread over time. > > > > On Fri, 2017-02-03 at 21:36 -0600, Rob Herring wrote: > >> The OF graph API leaves too much of the graph walking to clients when > >> in many cases the driver doesn't care about accessing the port or > >> endpoint nodes. The drivers typically just want the device connected via > >> a particular graph connection. of_graph_get_remote_node provides this > >> functionality. > >> > >> Signed-off-by: Rob Herring > >> --- > >> drivers/of/base.c | 28 ++++++++++++++++++++++++++++ > >> include/linux/of_graph.h | 8 ++++++++ > >> 2 files changed, 36 insertions(+) > >> > >> diff --git a/drivers/of/base.c b/drivers/of/base.c > >> index d4bea3c797d6..ea18ab16b92c 100644 > >> --- a/drivers/of/base.c > >> +++ b/drivers/of/base.c > >> @@ -2469,3 +2469,31 @@ struct device_node *of_graph_get_remote_port(const struct device_node *node) > >> return of_get_next_parent(np); > >> } > >> EXPORT_SYMBOL(of_graph_get_remote_port); > >> + > >> +struct device_node *of_graph_get_remote_node(const struct device_node *node, > >> + int port, int endpoint) > > > > I think this should have a documentation comment, similar to the > > of_graph_get_endpoint_by_regs one, as it is not really clear from the > > function name that the returned device node is the parent (or > > grandparent) device node containing the remote port to the specified > > node & port & endpoint. > > Also it might be interesting to the user that -1 is a wildcard value for > > port / endpoint. > > I really want to not allow using a wildcard here. Drivers should know > what port they want (or iterate over all of them). It didn't look like > any drivers were depending on the wildcard, but were just using -1 for > "no reg property" when really that should 0. Of course, I may have > missed something. > > I guess I could enforce port/endpoint > 0 here as there's no existing users. That sounds reasonable. If it works for all users, enforcing >= 0 should be fine, but in that case I'd change the parameters to be unsigned. regards Philipp