Re: Raspberry Pi support for Rev-2 & BL
paul_c <[email protected]> Fri, 18 Jan 2013 01:48:57 +0000
| Newsgroups | gmane.comp.sysutils.lcdproc |
|---|---|
| Message-ID | <[email protected]> |
Hi Markus The changes look OK to me except for the FIX-ME comments.. Comments like this have a habit of remaining in "production" code and once released, the incentive to resolve them tends to evaporate. On Thursday 17 January 2013, Markus Dolze wrote: > TODO > ==== > 1. There may be a memory leak in setup_io due to the pre-allocation > and alignment of 'gpio_mem'. The recent example code on [1] removed > this, too. Check if we can do the same. > [1] http://elinux.org/Rpi_Low-level_peripherals I used an early snippet of code to map the gpio in to memory. If the current references suggest that malloc is unnecessary, then make the change now. According to `man getpagesize' "Portable applications should employ sysconf(_SC_PAGESIZE)" as it has been labeled LEGACY and is architecture dependant in Linux - Either method (currently) works on a Pi and returns 4096 as expected. In response to the comment regarding gpio_map needing to be volatile, yes it does - It signals to the compiler that writes should not be optimised out or reordered. > 2. Check if we can get rid of the global variable 'gpio_map'. This could be made part of the driver struct, but as it is declared static, it is not visable outside hd44780-rpi.c nor is there any reason to make it available anywhere else. > 3. End user documentation. Documentation is not my strong point and I tend to write based on the assumption that the reader knows his/her way round source code. In conclusion, your rewite looks quite a bit better than my efforts to date. Regards, Paul.