VME userland API

Wolfgang Mauerer wolfgang.mauerer at siemens.com
Wed Jul 25 09:20:10 UTC 2012


On 24/07/12 23:53, Martyn Welch wrote:
> 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
okay.
> 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).
knowing the maximum number of windows is indeed not strictly required,
but it might still make sense to export these properties via sysfs.
Userspace could want this to use different strategies (e.g., don't
even try to allocate 6 windows if the driver support only 4).

Btw., what was the rationale behind the comment

/* XXX  We do not want to push aspace, cycle and width
 *      to userspace as they are
 */

that appears several times in vme_user? I don't see any
obvious reason why userspace should not know these properties,
but maybe I'm missing something.
> 
>> - 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?
sure. Luckily, there's a patch on the ML to do exactly this ;)
> 
>>       vme_user: Indentation cleanups
> 
> Think there may be a few more :-)
agreed... I'll add a final patch to correct the issues after
the series is finished.
> 
>>       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?
that could well be the case. A later patch reworks the state machine and
inserts breaks everywhere, I'll have to squash these patches together.
> 
>>       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.
okay.
> 
>>       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).
I'll use your change in later revisions.
> 
> The device model bits should probably be in the core rather than
> vme_user, wouldn't this functionality make sense for all drivers?
okay.
> 
>>       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)
the reason there's a nop function here is that I don't know if ca91c42x
can expose these status bits -- perhaps any of the experts on this
chipset can help out.

As I go along, I may be adding other pieces of information that are also
supported by ca91c42x, so I'll keep the function for the time being
(if it turns out there's really nothing that can be exported from this
bridge, the function will be removed)

> 
>>       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.
if it's okay to put it into vme.h, I'm fine with that. But since it's
shared information between kernel and userland, this would require
to make vme.h available for userland, which it is currently
not (vme_user would need need to be marked as header-y anyway).

We could also create a separate file with shared definitions that
is included by vme.h and vme_user.h. Any preferences?
> 
>>       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.
yeah, need to squash the patches appropriately.

> 
> There's definitely some good stuff here, look forward to seeing them
> polished :-)

Thanks, Wolfgang



More information about the devel mailing list