On Fri, Jan 13, 2017 at 05:28:37PM +0100, Stefano Babic wrote:
> Hi Paul,
>
> On 13/01/2017 16:39, Paul Gortmaker wrote:
> > [Adding Martyn to Cc]
> >
>
> Sorry, I forgot to run get_maintainer before posting :-)
>
No worries, and sorry for the delay, this ended up in my spam filter :-/
> > [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 <paul.gortma...@windriver.com>
> >> 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.
>
>
Yup, I agree - we need to re-instate this function, it's called when a
device driver is removed.
Thanks for noticing this, it's been a while since I wrote this code and
the patch /looked/ sane...
Martyn
> >
> >>
> >>
> >> 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: sba...@denx.de
> =====================================================================
