search

sensorplatforms / open-sensor-platform

Open Sensor Platform project
Apache License 2.0
61 stars 37 forks source link

Adding CMSIS HAL for GPIO Driver #6

Open pankajgoenka opened 8 years ago

pankajgoenka commented 8 years ago

Added CMSIS HAL for GPIO Driver Modified the SH-Xpresso-LPC54102 project to include HAL files

Signed-off-by: Pankaj Goenka pgoenka@audience.com

pankajgoenka commented 8 years ago

Driver_GPIO.c - For the pinmuxing[] definition as I had mentioned in my original TODO comment that the pins should be configured in the respective modules - e.g. I2C pins must be initialized in I2C driver file and not in Driver_GPIO.c. Remember that Driver_GPIO.c is a generic interface for accessing GPIO functions and not meant to initialize anything in particular! The application code should use these 'CMSIS' GPIO APIs now to initialize any platform specific GPIO pins.

I2C pins were already being initialized from I2C module so removed the redundant initialization. Sensor pin initialization is now moved to the sensor interface initialization function Board_SensorIfInit().

Driver_GPIO.h - I think the file copyright header should either be ARM or OSP. Because the Pinecone work is derived from ARM it maybe better to keep ARM's original copyright notice. If Pinecone has issues with that we can fix it!

As discussed, this file is provided by Pinecone and used as it is so can't modify it.

hostif_i2c.c - Where is ENCODE_PORT_PIN macro coming from? Its not in any Driver_GPIO files.

Yes ideally this macro should be defined in Driver_GPIO.h. However, as discussed, this file is provided by Pinecone and we are not making any change in this file. Therefore retaining the definition in hw_setup_xpresso_lpc54102.h.

Driver_GPIO.c GPIOx_Initialize(): Why DMA initialization is happening here - this has nothing to do with a Generic GPIO driver interface. Please be mindful when copy-pasting stuff! And like I said in previous comments - the pinmuxing setup should not happen here. It shoud remain in hw_setup.c or better be moved to/referenced from respective modules that use those pins.

Agreed. Retaining DMA initialization in hw_setup.c and moving pinmux initialization to individual modules.

hw_setup_xpresso_lpc54102.h - Note that the following declarations/defines should not be under the heading "DIAGNOSTIC LED/GPIO INTERFACE":

define NC (uint32_t)0xFFFFFFFF

typedef uint32_t PinName;

define ENCODE_PORT_PIN(port,pin) (PinName)(((uint32_t)port << 16) + (uint16_t)pin)

define DECODE_PORT(X) (((uint32_t)(X) >> 16) & 0xF)

define DECODE_PIN(X) ((uint32_t)(X) & 0xFFFF)

Also some of these should be part of Driver_GPIO.h if they are being used for GPIO interfacing.

Agreed. Moved under correct sections

main.c - I believe by now it must be clear that Driver_GPIO.Initialize() is not a replacement for SystemGPIOConfig(), rather the Driver_GPIO.Initialize should be called first thing from SystemGPIOConfig() function where certain command GPIOs are being initialized (e.g. LED and Diagnostic ones that don't belong to particular driver module)

Retaining SystemGPIOConfig() which is called from main(). It initializes DMA after calling GPIO Init.