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.