Jeff Epler wrote: > Please feel free to ream the code -- I didn't have any familiarity with > the hm2 driver going in, so I'm sure I got things wrong. I'd sure take > your understanding over mine when it comes to the memory allocation > issue. Here's the part where I think I started to go off the rails: I > added a call which sets up encoder.control_reg as a tram read region. I > think I intended to get rid of the kmalloc of the same memory region, > but obviously I didn't.
Sure, no problem. I should have warned you how rough the hm2 code is at the moment, the internal interfaces and invariants are still in flux and only partially described by comments sprinkled throughout the code. I want to try to clean this up and document it a bit to make it easier for others to work on the code, though it looks like you picked it right up. > As for the actual workings of the encoder index, the register needs to > be read at least when index-enable is true in HAL, so that emc can react > on the cycle that the fpga clears the bit to indicate that an index was > seen. It needs to be written whenever index-enable goes true in HAL, or > when the other bits change--but index-enable is the only one normally > changing during machine operation. > > So on the one hand it's not something like a prescaler register that is > generally only used in startup code, and the most straightforward thing > to do is just read it all the time. On the other hand, not everybody > will actually use the index with every encoder. > > So I guess the question becomes whether it's better to make everyone pay > for an additional 32-bit read for every encoder every cycle, or to make > those who use index pay for an additional 32-bit read and an address > cycle (I think?) when index is actually enabled. Again, you have the > expertise here so I'm happy to let you make the call. (it's probably not > a big consideration on 5i20 anyway..) You have the trade-off exactly right, and I too am not sure what the best way to go here is. I think I sort of understand when you'd use the encoder index, but i dont understand when you'd use the index mask. One of the stinky things about the hm2 driver right now is that there is no good mechanism to do load-time configuration of the driver. If there was a way for the user to say, about each encoder, whether they want to use the index line and whether they want to use an index-enable line, then that would make things better - whichever way the decision above goes, you only have to pay for it on encoders where you *might* actually want it. JMK and SWP started doing kind of configuration by adding an "r" chunk to the bitfile, I'm going to try to do it by expanding the "config" load-time parameter. > I hope you are able to salvage something from this work I did. It looks good, thanks for the contribution. I hope I didnt come off sounding too critical, that was not my intent. -- Sebastian Kuzminsky Computer Science for life, that's my direction Instead of b-balls, my homies throw exceptions -- MC Plus+ ------------------------------------------------------------------------- This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100&url=/ _______________________________________________ Emc-developers mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/emc-developers
