Skip to content

Adding CMSIS HAL for GPIO Driver#6

Open
pankajgoenka wants to merge 4 commits intomasterfrom
feature-hal-gpio
Open

Adding CMSIS HAL for GPIO Driver#6
pankajgoenka wants to merge 4 commits intomasterfrom
feature-hal-gpio

Conversation

@pankajgoenka
Copy link
Contributor

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
Copy link
Contributor Author

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.

pankajgoenka added a commit that referenced this pull request Dec 11, 2015
Moved pinmux initialization to respective module function
Moved GPIO macros and types under correct heading in the code
Closed all review comments from pull request #6

Signed-off-by: Pankaj Goenka <pgoenka@audience.com>
Modified the SH-Xpresso-LPC54102 project to include HAL files

Signed-off-by: Pankaj Goenka <pgoenka@audience.com>
Moved pinmux initialization to respective module function
Moved GPIO macros and types under correct heading in the code
Closed all review comments from pull request #6

Signed-off-by: Pankaj Goenka <pgoenka@audience.com>
Moved Driver_GPIO.c to embedded/Drivers/LPC5410x-CM4/CMSIS
Removed private structure GPIO_T
Removed GPIO_AINConfig()
Added Chip_PININT_Init() to GPIO Init
Removed calls to Chip_PININT_ClearIntStatus & Chip_INMUX_PinIntSel from OSP_GPIO_PowerControl
Added comment describing how port and pin number are encoded
Using Chip_GPIO_WriteDirBit, Chip_GPIO_ReadDirBit functions rather than directly modifying registers
Made the description and usage of 'pin' argument in OSP_GPIO_SetTrigger in line with the other GPIO HAL APIs
Asserting that the value of 'val' received in OSP_GPIO_WritePin is <=1
Returning ARM_DRIVER_ERROR_UNSUPPORTED from OSP_GPIO_SetHandler

Signed-off-by: Pankaj Goenka <pgoenka@audience.com>
* Made SetTrigger's pin parameters inline with parameters of other APIs
* Closed coding guidelines related comments
* Added calls to Driver_GPIO.SetTrigger

Signed-off-by: Pankaj Goenka <pgoenka@audience.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants