From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752116AbdAMQ2t (ORCPT ); Fri, 13 Jan 2017 11:28:49 -0500 Received: from mail-out.m-online.net ([212.18.0.9]:59167 "EHLO mail-out.m-online.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750906AbdAMQ2r (ORCPT ); Fri, 13 Jan 2017 11:28:47 -0500 Subject: Re: VME: devices not removed after commit 050c3d52cc7 To: Paul Gortmaker , Stefano Babic References: <54f7a6d6-f04d-b13a-04bf-b7e4405d2619@denx.de> <20170113153925.GU11537@windriver.com> Cc: linux-kernel@vger.kernel.org, Martyn Welch From: Stefano Babic Message-ID: <19155f10-7190-0cf6-6ea7-43956dcfc173@denx.de> Date: Fri, 13 Jan 2017 17:28:37 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <20170113153925.GU11537@windriver.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Paul, On 13/01/2017 16:39, Paul Gortmaker wrote: > [Adding Martyn to Cc] > Sorry, I forgot to run get_maintainer before posting :-) > [VME: devices not removed after commit 050c3d52cc7] On 13/01/2017 (Fri 11:03) Stefano Babic wrote: > >> Hi, >> >> I have updated a custom VME device driver (mainly based on vme_user.c) >> to 4.9 (previously it was for 3.14-). >> >> I see that VME device drivers cannot be loaded and unloaded due to this >> commit: >> >> commit 050c3d52cc7810d9d17b8cd231708609af6876ae >> Author: Paul Gortmaker >> Date: Sun Jul 3 14:05:56 2016 -0400 >> >> vme: make core vme support explicitly non-modular > > I've gone back and looked at this, and vme_user.c and I'm not yet 100% > convinced this is the right conclusion. But perhaps, and I've put > Martyn on the Cc, in the hopes that he can clarify as well, if needed. Thanks. What I am seeing is that (*remove) in bus_type is called when a device is removed from the bus, and not when the bus is removed. This looks consistent with other busses. And in fact, the function was: static int vme_bus_remove(struct device *dev) { int retval = -ENODEV; struct vme_driver *driver; struct vme_dev *vdev = dev_to_vme_dev(dev); driver = dev->platform_data; if (driver->remove != NULL) retval = driver->remove(vdev); So this is the point where the remove for the VME's device is called, as far as I understand. > >> >> >> In fact, this drops the remove function, that scans all devices attached >> to the bus and call their remove function. > > So I guess my confusion here is between removal of a VME device, vs. the > removal of a complete VME bus. Right, this is what must be cleared. In my understanding, the dropped remove function is called when a device is removed from the bus, that leads to the fact that the VME's device is not cleaned unloaded. > The above commit you reference was based > on the premise that removal of a VME bus is not supported. Agree, and I fully agree that loading / unloading of VME makes less sense. > Which is not > to say that a VME device removal is not supported. I agree to reach this goal - just the dropped remove() is called IMHO when a device is dropped from the VME bus and not when the bus is removed from system. This is what we need to clarify here. > >> >> That means that "remove" entry points in VME device driver (let see in >> drivers/staging/vme/devices/vme_user.c) are now dead code and the >> required cleanup code is not called at all (devices and class are not >> removed). Reloading the same driver cause errors due to the missing >> cleanup by unloading. This does not let build VME device drivers as >> module, as it is supposed to be done. > > Again, I don't think this analysis is 100% right, but I can't be sure > because your driver is out of tree and I don't know what it does > precisely. Looking at vme_user.c example, it has its own .remove > function that should be executed at module unload, and that would do all > the cleanup (see vme_user_remove). In my test, vme_user's remove is never called with the patch applied. Reverting the patch, it works again, and remove is called: loading / unloading of VME's device drivers works again. > >> >> Paul, what do you mind ? > > For sure, we can restore the .remove and vme_bus_remove portions of that > commit if it is a real regression against a correct use of the > infrastructure, I absolutely agree that we have to clarify the point before doing something. > but I'm still not clear how you'd be triggering the > vme_bus_remove unless the vme device driver was going up into its > parent's bus struct directly. No, this is not done ! > Maybe Martyn can spot where I've > misunderstood the bus vs. device separation here. > Best regards, Stefano -- ===================================================================== DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de =====================================================================