Re: [patch 2/5] Staging: vme: add VME userspace driver

From: Martyn Welch
Date: Mon Aug 10 2009 - 12:29:30 EST


Emilio G. Cota wrote:
Martyn Welch wrote:
Instead of using that we implemented a heretic IOCTL-based
interface for user-space; at least with it you could create a
[ snip ]
#define VME_IOCTL_START_DMA _IOWR('V', 10, struct vme_dma)

I am moving the interface in that direction, I remain unconvinced about the contents of your vme_mapping structure, it's too tsi-148 specific.

Could you please point out why is too tsi148-specific?

The point here is that the driver should know *nothing* about
windows, etc. What it should just know is:
- I want a mapping of a certain size to VME address X

I'm not convinced. Given that each bridge provides a limited number of windows (some more than others), we are limited to how large a window we can produce (we need to map them somewhere) and the potential combinations are so great (independant 16, 32, 40, 64 and CR/CSR address spaces, not to mention access modes) it is important to assign the windows to a driver, so that it may move them as it sees fit. For example, supporting 10 devices (as you have mentioned earier) with a single driver could potentially require a single window that it knows it has exclusive use of to position over each devices register as required without either having to provide a large window (unfeasibly large on most/all platforms if they are scattered across the 64-bit address space) or needing more windows than are available on any of the bridges I have seen (8 being the maximum).

The struct provides exactly this.
Ah - vme_dma does to some degree. I was talking about vme_mapping for configuring vme windows, from your vmebus.h:

115 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l115> * This data structure is used for describing both a hardware window
116 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l116> * and a logical mapping on top of a hardware window. Therefore some of
117 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l117> * the fields are only relevant to one of those two entities.
118 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l118> */
119 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l119> struct vme_mapping {
120 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l120> int window_num;
121 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l121>

122 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l122> /* Reserved for kernel use */
123 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l123> void *kernel_va;
124 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l124>
125 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l125> /* Reserved for userspace */
126 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l126> void *user_va;
127 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l127> int fd;
128 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l128>

129 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l129> /* Window settings */
130 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l130> int window_enabled;
131 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l131> enum vme_data_width data_width;
132 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l132> enum vme_address_modifier am;
133 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l133> int read_prefetch_enabled;
134 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l134> enum vme_read_prefetch_size read_prefetch_size;

Tsi-148 specific.

135 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l135> enum vme_2esst_mode v2esst_mode;
136 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l136> int bcast_select;
137 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l137> unsigned int pci_addru;
138 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l138> unsigned int pci_addrl;

Why are these split, why not a single unsigned long long?

139 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l139> unsigned int sizeu;
140 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l140> unsigned int sizel;

Ditto.

141 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l141> unsigned int vme_addru;
142 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l142> unsigned int vme_addrl;

Ditto.

143 <http://repo.or.cz/w/tsi148vmebridge.git?a=blob;f=driver/vmebus.h;h=6f25a7715a973ae07a6d1f71bdb15d638a56e7aa;hb=HEAD#l143> };

In addition your enum vme_address_modifier would need extending when specifying slave windows, the tsi-148 can support windows with USER and SUP access, as well as DATA and PRG. Why throw access privileges and address spaces together like that? The VME spec also specifies 4 User definable address spaces...

I'm still not convinced by all these structures - you've defined tonnes of them, I don't feel that it aids readability and maintainability at all.

Also, could you please send me (off-list) documentation of the
Universe bridge? It'd be useful for the work on the generic layer.

It's all over the web :-) We've had it for years and I'm not sure under what terms we originally got it, so I'm afraid I've got to assume we're still bound under some NDA, sorry. Searching google for "Universe II Manual" much get you what you want though...

Martyn

E.



--
Martyn Welch MEng MPhil MIET (Principal Software Engineer) T:+44(0)1327322748
GE Fanuc Intelligent Platforms Ltd, |Registered in England and Wales
Tove Valley Business Park, Towcester, |(3828642) at 100 Barbirolli Square,
Northants, NN12 6PF, UK T:+44(0)1327359444 |Manchester,M2 3AB VAT:GB 927559189
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/