From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752282AbdBCKbT (ORCPT ); Fri, 3 Feb 2017 05:31:19 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:49268 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752198AbdBCKbS (ORCPT ); Fri, 3 Feb 2017 05:31:18 -0500 Date: Fri, 3 Feb 2017 11:31:21 +0100 From: Greg KH To: Laurentiu Tudor Cc: "devel@driverdev.osuosl.org" , "linux-kernel@vger.kernel.org" , "agraf@suse.de" , "arnd@arndb.de" , Ioana Ciornei , Ruxandra Ioana Radulescu , Bharat Bhushan , Stuart Yoder , Catalin Horghidan , Leo Li , Roy Pledge Subject: Re: [PATCH 2/9] staging: fsl-mc: fix device ref counting Message-ID: <20170203103121.GA30656@kroah.com> References: <20170201114329.21276-1-laurentiu.tudor@nxp.com> <20170201114329.21276-3-laurentiu.tudor@nxp.com> <20170203095645.GE24767@kroah.com> <589458CF.90804@nxp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <589458CF.90804@nxp.com> User-Agent: Mutt/1.7.2 (2016-11-26) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Feb 03, 2017 at 10:17:53AM +0000, Laurentiu Tudor wrote: > Hi Greg, > > Thanks for having a look. Comment below. > > On 02/03/2017 11:56 AM, Greg KH wrote: > > On Wed, Feb 01, 2017 at 05:43:22AM -0600, laurentiu.tudor@nxp.com wrote: > >> From: Laurentiu Tudor > >> > >> Drop unneeded get_device() call at device creation > >> and, as per documentation, drop reference count > >> after using device_find_child() return. > >> > >> Signed-off-by: Laurentiu Tudor > >> --- > >> drivers/staging/fsl-mc/bus/dprc-driver.c | 1 + > >> drivers/staging/fsl-mc/bus/fsl-mc-bus.c | 1 - > >> 2 files changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/staging/fsl-mc/bus/dprc-driver.c b/drivers/staging/fsl-mc/bus/dprc-driver.c > >> index 4e416d8..e4b0341 100644 > >> --- a/drivers/staging/fsl-mc/bus/dprc-driver.c > >> +++ b/drivers/staging/fsl-mc/bus/dprc-driver.c > >> @@ -188,6 +188,7 @@ static void dprc_add_new_devices(struct fsl_mc_device *mc_bus_dev, > >> child_dev = fsl_mc_device_lookup(obj_desc, mc_bus_dev); > >> if (child_dev) { > >> check_plugged_state_change(child_dev, obj_desc); > >> + put_device(&child_dev->dev); > >> continue; > >> } > >> > >> diff --git a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c > >> index cc20dc4..7c6a43b 100644 > >> --- a/drivers/staging/fsl-mc/bus/fsl-mc-bus.c > >> +++ b/drivers/staging/fsl-mc/bus/fsl-mc-bus.c > >> @@ -537,7 +537,6 @@ int fsl_mc_device_add(struct dprc_obj_desc *obj_desc, > >> goto error_cleanup_dev; > >> } > >> > >> - (void)get_device(&mc_dev->dev); > > > > This implies that your device reference counting is totally wrong and > > messed up. Does this fix anything? Break anything? It should do > > something different now... > > It fixes the refcounting in the sense that I'm now seeing the error > that i think you were referring to in your previous reviews, > when we hot unplug a device: > > "Device 'foo.N' does not have a release() function, it is broken and > must be fixed." > > See next patch that adds the required callback. > > Regarding this particular get_device(), i have no clue why the > original author placed it here. I've looked over other bus > implementations and didn't see something similar. Ah, that makes more sense, thanks. I've now applied this and the next patch and will wait for you to respin the rest. thanks, greg k-h