> -----Original Message----- > From: Thomas Gleixner [mailto:tglx@xxxxxxxxxxxxx] > Sent: Wednesday, May 18, 2016 6:12 PM > To: Sell, Timothy C > Cc: mingo@xxxxxxxxxx; dave.hansen@xxxxxxxxxxxxxxx; > timur@xxxxxxxxxxxxx; galak@xxxxxxxxxxxxxxxxxxx; Kershner, David A; > corbet@xxxxxxx; mingo@xxxxxxxxxx; hpa@xxxxxxxxx; Arfvidson, Erik; > hofrat@xxxxxxxxx; dzickus@xxxxxxxxxx; Curtin, Alexander Paul; > janani.rvchndrn@xxxxxxxxx; sudipm.mukherjee@xxxxxxxxx; > prarit@xxxxxxxxxx; Binder, David Anthony; nhorman@xxxxxxxxxx; > dan.j.williams@xxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux- > doc@xxxxxxxxxxxxxxx; driverdev-devel@xxxxxxxxxxxxxxxxxxxxxx; *S-Par- > Maintainer; Greg KH; Jes Sorensen > Subject: Re: new driver for drivers/virt/? > > On Wed, 18 May 2016, Sell, Timothy C wrote: > > We have a bus driver currently in drivers/staging/unisys/visorbus/ that > > we are trying to get out of staging and into the kernel proper. Since > > "visorbus" is a driver to host a virtual bus presented to a Linux guest > > in a hypervisor environment (refer to > > drivers/staging/unisys/Documentation/overview.txt for more details), > > Greg KH and Jes Sorensen have suggested the possibility that drivers/virt/ > > might be a good place for visorbus. But right now, we see that the only > > driver under drivers/virt/ is the Freescale hypervisor environment, which > > made us wonder whether this was really the correct place. > > > > Would you have any guidance for us? > > Our intent is to push our visorbus out of staging immediately following > > the current merge window. > > What's the problem with Gregs and Jes suggestion? I don't see any. > That's good; glad you agree with them. We just wanted to double-check with those of you listed as maintainers of drivers/virt/. Thanks. > There is bigger fish to fry than the final place of this driver. I had just a > peek at the staging code and there is enough stuff which wants to be > cleaned > up before moving anywhere. I don't have time to do a proper review now, > but > here are a few hints upfront: > > 1) Locking: > > visordriver_callback_lock: > > That should be a mutex, not a semaphore > > periodic_work->lock: > > Why is this a rw_lock if it's only locked with write_lock? And what's > the purpose of this lock at all? > > 2) Memory barriers: > > Completely undocumented wmb()s without corresponding rmb()s to do > obscure > protection of that periodic work stuff. > > 3) periodic_work: > > That set of functions is obscure. Especially visor_periodic_work_stop() > makes me shudder. See also #2. > > That work->lock does not inspire my confidence further. > > 4) Exports: > > A gazillion of exports which are just wrappers around another set of > exports > > 5) Function comments: > > Try to mimic kerneldoc comments, i.e. start with: /** > but do not implement any of the kerneldoc requirements. > We'll take a look at these. Thanks. Tim Sell > I'll try do find a time slot for a proper review of that thing, but don't > expect that to happen in the next days. > > Thanks, > > tglx > _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
