Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754033AbdHYAKe (ORCPT ); Thu, 24 Aug 2017 20:10:34 -0400 Received: from ozlabs.org ([103.22.144.67]:50061 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753456AbdHYAKc (ORCPT ); Thu, 24 Aug 2017 20:10:32 -0400 From: Michael Ellerman To: Rob Herring Cc: "devicetree\@vger.kernel.org" , linuxppc-dev , "linux-kernel\@vger.kernel.org" , Frank Rowand , Benjamin Herrenschmidt , Paul Mackerras , David Laight Subject: Re: [PATCH v2 2/5] powerpc: pseries: vio: match parent nodes with of_find_node_by_path In-Reply-To: References: <20170821151651.25096-1-robh@kernel.org> <20170821151651.25096-3-robh@kernel.org> <87fuckb2z2.fsf@concordia.ellerman.id.au> User-Agent: Notmuch/0.21 (https://notmuchmail.org) Date: Fri, 25 Aug 2017 10:10:30 +1000 Message-ID: <87mv6ov761.fsf@concordia.ellerman.id.au> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3174 Lines: 74 Rob Herring writes: > On Tue, Aug 22, 2017 at 12:12 AM, Michael Ellerman wrote: >> Rob Herring writes: >> >>> In preparation to remove the full path from device_node.full_name, use >>> of_find_node_by_path instead of open coding with strcmp. >>> >>> Signed-off-by: Rob Herring >>> Cc: Benjamin Herrenschmidt >>> Cc: Paul Mackerras >>> Cc: Michael Ellerman >>> Cc: linuxppc-dev@lists.ozlabs.org >>> --- >>> v2: >>> - rebased to linux-next and removed spurious change fro patch 1. >>> >>> arch/powerpc/platforms/pseries/vio.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c >>> index aa5ca74316fa..5754572deb23 100644 >>> --- a/arch/powerpc/platforms/pseries/vio.c >>> +++ b/arch/powerpc/platforms/pseries/vio.c >>> @@ -1357,9 +1357,9 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node) >>> */ >>> parent_node = of_get_parent(of_node); >>> if (parent_node) { >>> - if (!strcmp(parent_node->full_name, "/ibm,platform-facilities")) >>> + if (parent_node == of_find_node_by_path("/ibm,platform-facilities")) >>> family = PFO; >>> - else if (!strcmp(parent_node->full_name, "/vdevice")) >>> + else if (parent_node == of_find_node_by_path("/vdevice")) >>> family = VDEVICE; >> >> This leaks references to the looked up nodes. >> >> Both these nodes are defined in PAPR (our hypervisor spec), and both of >> them must have a device_type, either "ibm,platform-facilities" or >> "vdevice". >> >> Looking at the commit that added the code I don't see any particular >> reason it used the comparison against full_name, rather than using the >> device_type. >> >> So I'm inclined to do this instead: >> >> diff --git a/arch/powerpc/platforms/pseries/vio.c b/arch/powerpc/platforms/pseries/vio.c >> index 8a47f168476b..f26f906e6021 100644 >> --- a/arch/powerpc/platforms/pseries/vio.c >> +++ b/arch/powerpc/platforms/pseries/vio.c >> @@ -1357,9 +1357,9 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node) >> */ >> parent_node = of_get_parent(of_node); >> if (parent_node) { >> - if (!strcmp(parent_node->full_name, "/ibm,platform-facilities")) >> + if (!strcmp(parent_node->type, "ibm,platform-facilities")) >> family = PFO; >> - else if (!strcmp(parent_node->full_name, "/vdevice")) >> + else if (!strcmp(parent_node->type, "vdevice")) >> family = VDEVICE; >> else { >> pr_warn("%s: parent(%s) of %s not recognized.\n", >> >> >> I've checked both Qemu and kvmtool add the device_type, and I'm fairly >> confident that PowerVM does too. Anyway I'll test it on all the machines >> I can find. > > Okay, do you want me to respin the patch or will you update it with I merged it. Should be in next today. cheers