On 24/07/12 16:34, Wolfgang Mauerer wrote: > On 20/07/12 10:48, Martyn Welch wrote: >> On 19/07/12 23:39, Greg Kroah-Hartman wrote: >>> On Thu, Jul 19, 2012 at 08:08:50PM +0100, Martyn Welch wrote: > >> Still in two minds about whether we should keep vme_user around. It does seem >> to have users and there are simple drivers/tasks that could be coded in user >> space using this. On the other hand, the interface to user space it currently >> provides isn't idea, neither is the fact that it grabs 4 VME windows when the >> module is loaded, regardless of whether they ever get used. It was certainly >> useful during development of the VME subsystem, but my focus is really on the >> subsystem and kernel space VME drivers. > > vme_user is useful when it comes to porting legacy VME code > to Linux. Having a possibility to access the bus from userland will > surely ease this task, userland is simply more convenient to work > with. If speed is not too important, there's no real need to > involve all the kernel complexity. > > Besides the sub-optimal interface, there are some functionalities that > should be exported in addition to what is currently available from > vme_user. I'm working on cleaning up the driver, and would suggest to > adapt the interface as follows: > > - Support more than one bridge; place the device files in > /dev/bus/vme/000/... etc., similar to the USB subsystem Definitely. > - Dynamically query the number of available master/slave windows > from the low-level driver, expose this information to userland, > and don't create access windows on startup, but only when needed. The driver definitely needs to stop grabbing windows at load. The framework expects the drivers to request resources when required, rather than querying the number of available windows, I'd prefer that the user space driver/application request a window, with some mechanism to tell whether the request had succeeded. I'm not sure there's any point in a function that tells you how many are at any given point in tie, that information could be wildly wrong by the time a request for one is made. The request would probably need to include the characteristics required as not all windows are equal (only some slave windows can do A16 in certain chipsets, some chipsets can't do A64 at all, etc). > - Expose the RMW mechanism and semaphores to userland > - Implement a timeout mechanism when waiting for IACKs That would need to be implemented in the drivers, then exposed through the kernel API. > - Provide generic/status information (e.g., sysfail, slot id, ...), via > some mechnism (ioctl, sysfs, whatever) > - Possibly map location monitors into userland (interrupt proagation > via uio) > > Any comments on these ideas? > > A pull request for some changes in this direction is given below. > However, the code is _NOT_ meant to be pulled right now -- the patches > are not yet in shape; some things still need to be implemented and/or > polished. But it could simplify the discussion. > > Unfortunately, I cannot do any real tests for a temporary lack of > hardware at the moment. But this should not really concern a general > discussion about the future shape of this driver. > > Best, Wolfgang > > #################################################################### > The following changes since commit 28a33cbc24e4256c143dce96c7d93bf423229f92: > > Linux 3.5 (2012-07-21 13:58:29 -0700) > > are available in the git repository at: > git://github.com/siemens/vme.git not-yet-for-upstream > Few comments below: > Wolfgang Mauerer (16): > Tiny comment fix > Extract VME major number definition from the device driver > vme_user: Clarify buffer allocation comment > vme_user: Get rid of Universe II references > vme_user: Cosmetic fixes > vme_user: Some more context for error messages Use pr_err and friends? > vme_user: Indentation cleanups Think there may be a few more :-) > vme_user: Push ioctl arguments into local scope This patch removes a "break;", I might be wrong (just looking at the patch in isolation) but isn't it required? > vme_user: Add support for sending RMW cycles > vme: Equip irq generation with a timeout I appreciate that you won't have access to all the bridges, but I think the ca91cx42 driver needs to be changed at least test to see if a timeout has been provided and error/throw a warning if it has. > vme_user: Use device_type methods to map per-bridge information into userland The function to retrieve the bus enumeration is very similar to part of the patch I proposed earlier in this thread (I think you need to check the pointers not null). The device model bits should probably be in the core rather than vme_user, wouldn't this functionality make sense for all drivers? > vme_user: Export slot id to userland Looks good (as long as we are going the IOCTL route). > tsi148: Allow for exporting the bridge status Looks Ok. > ca91c42x: Allow for exporting status information Don't add the function if it's not supported - have the framework just return -EINVAL if the pointer is null (which it already does) > vme, vme_user: Make bridge status public Either the status structure should be in vme.h (so as not to need vme_user.h in the core) or the sysfail and acfail should be retrievable without the structure and the structure is a vme_user only thing. > vme_user: Simplify ioctl state machine I think the pr_fmt added here would be of benefit in the "vme_user: Some more context for error messages" patch. There's definitely some good stuff here, look forward to seeing them polished :-) Martyn > > drivers/staging/vme/devices/vme_user.c | 323 ++++++++++++++++++++------------ > drivers/staging/vme/devices/vme_user.h | 24 ++- > drivers/vme/bridges/vme_ca91cx42.c | 10 +- > drivers/vme/bridges/vme_tsi148.c | 43 ++++- > drivers/vme/vme.c | 30 +++- > drivers/vme/vme_bridge.h | 6 +- > include/linux/major.h | 2 + > include/linux/vme.h | 8 +- > 8 files changed, 311 insertions(+), 135 deletions(-) > > -- > Siemens AG, Corporate Technology > Corporate Competence Centre Embedded Linux > _______________________________________________ > devel mailing list > devel@xxxxxxxxxxxxxxxxxxxxxx > http://driverdev.linuxdriverproject.org/mailman/listinfo/devel > _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
