vsergeev / python-periphery

A pure Python 2/3 library for peripheral I/O (GPIO, LED, PWM, SPI, I2C, MMIO, Serial) in Linux.
MIT License
529 stars 139 forks source link

MMIO class cannot perform true 16-bit/32-bit accesses #3

Closed marcan closed 7 years ago

marcan commented 8 years ago

mmio.py uses ctypes to read/write 16- and 32-bit quantities. Unfortunately, although common sense would be that those wrappers do real 16- and 32-bit loads and stores, that's not the case. The wrappers use memcpy, and worse, actually do read/modify/write for stores:

https://github.com/python/cpython/blob/12417ee0a6834cb874efd8061ca2d5172a0a9f42/Modules/_ctypes/cfield.c#L854

This breaks MMIO with registers that have side effects when read/written, and potentially breaks even worse on platforms where memcpy doesn't know how to coalesce writes and it ends up doing the write bytewise. mmio.py needs to find some other way to read/write registers that actually uses a single-instruction read or write.

vsergeev commented 8 years ago

Thanks for looking into this and isolating the root cause. This is definitely a serious issue for the MMIO module. I'll think about the options that don't involve compiling and binding C code.

mpb27 commented 8 years ago

Changed the write32() to perform self.mapping[offset:offset+4] = struct.pack("<L", value) and that eliminates the read, then write issue (on my Cortex A9 platform).

I borrowed the code/idea from https://graycat.io/tutorials/beaglebone-io-using-python-mmap/

marcan commented 7 years ago

FWIW, you should do the same thing for reads. Though the impact is smaller, registers that are read-sensitive do exist and doing a bytewise memcpy from them will break things.

vsergeev commented 7 years ago

@marcan I modified reads to use the same kind of access through self.mapping[...] in 3944894957ebc0695f166acc5cba817da6dc9500 -- is that what you mean?

marcan commented 7 years ago

Yes, sorry, I hadn't seen that as it wasn't linked from the issue. Assuming this access method results in true 32-bit or 16-bit operations for reads/writes then all is good.

vsergeev commented 7 years ago

@marcan OK cool, no problem. I don't have a jig to figure out what kind of instructions are being generated, but this should still be better than it was before.

mpb27 commented 7 years ago

I can confirm that at least the write32() results in one 32-bit transaction on my bus. My application is on a ZYNQ where you can implement custom peripherals in FPGA logic.

vsergeev commented 7 years ago

@mpb27 great, thanks for testing. Any chance you are setup to observe read32() with one of your PL peripherals?