Hi Duc,
As I mentioned in the other patch, please save and consolidate all
patch revisions and send an entire new patch set of -v2 patches with
all your changes. You can also squash changes into your patch set, for
example to address reviewer comments like these. We would like to have
a clean set of patches to apply to RTEMS, which may not reflect how
you arrived at the end result of your code. I will try to discuss more
in today's discord meeting.
Gedare
On Wed, Jul 6, 2022 at 5:09 AM Duc Doan <dtbpk...@gmail.com> wrote:
>
> Hello Cedric,
>
> Thank you for your feedback. I agree with you that there are places that
> could be optimized out. Here is a new patch for that.
>
> Best,
>
> Duc Doan
>
> ---
> bsps/arm/stm32f4/gpio/gpio.c | 87 ++++++++++++------------------------
> 1 file changed, 29 insertions(+), 58 deletions(-)
>
> diff --git a/bsps/arm/stm32f4/gpio/gpio.c b/bsps/arm/stm32f4/gpio/gpio.c
> index e971f91140..b632236d8d 100644
> --- a/bsps/arm/stm32f4/gpio/gpio.c
> +++ b/bsps/arm/stm32f4/gpio/gpio.c
> @@ -22,9 +22,18 @@
> #include <stdlib.h>
>
> /*********** Helpers *****************/
> -static stm32f4_gpio *get_gpio_from_base(
> - rtems_gpio *base
> -);
> +/**
> + * @brief Macro to get stm32f4_gpio object from a base rtems_gpio
> + * object.
> + *
> + * This is a wrapper of RTEMS_CONTAINER_OF macro
> + *
> + * @param base The pointer to a rtems_gpio object
> + * @retval The pointer to the stm32f4_gpio object owning
> + * the specified rtems_gpio object
> + */
> +#define get_gpio_from_base(base) \
> + RTEMS_CONTAINER_OF(base, stm32f4_gpio, base)
>
> /*********** GPIO API ***************/
> static rtems_status_code stm32f4_gpio_get(
> @@ -115,44 +124,6 @@ static GPIO_TypeDef *GPIOx[] = {
> #endif /* STM32F429X */
> };
>
> -static uint16_t GPIO_PIN_x[] = {
> - GPIO_PIN_0,
> - GPIO_PIN_1,
> - GPIO_PIN_2,
> - GPIO_PIN_3,
> - GPIO_PIN_4,
> - GPIO_PIN_5,
> - GPIO_PIN_6,
> - GPIO_PIN_7,
> - GPIO_PIN_8,
> - GPIO_PIN_9,
> - GPIO_PIN_10,
> - GPIO_PIN_11,
> - GPIO_PIN_12,
> - GPIO_PIN_13,
> - GPIO_PIN_14,
> - GPIO_PIN_15
> -};
> -
> -static uint32_t LL_EXTI_LINE_x[] = {
> - LL_EXTI_LINE_0,
> - LL_EXTI_LINE_1,
> - LL_EXTI_LINE_2,
> - LL_EXTI_LINE_3,
> - LL_EXTI_LINE_4,
> - LL_EXTI_LINE_5,
> - LL_EXTI_LINE_6,
> - LL_EXTI_LINE_7,
> - LL_EXTI_LINE_8,
> - LL_EXTI_LINE_9,
> - LL_EXTI_LINE_10,
> - LL_EXTI_LINE_11,
> - LL_EXTI_LINE_12,
> - LL_EXTI_LINE_13,
> - LL_EXTI_LINE_14,
> - LL_EXTI_LINE_15
> -};
> -
> static unsigned int EXTIx_IRQn[] = {
> EXTI0_IRQn,
> EXTI1_IRQn,
> @@ -200,13 +171,13 @@ static unsigned int EXTIx_IRQn[] = {
> * @brief Converts pin number from 0-15 to HAL pin mask.
> * @param pin is the pin number from 0-15
> */
> -#define STM32F4_GET_HAL_GPIO_PIN(pin) (GPIO_PIN_x[( pin )])
> +#define STM32F4_GET_HAL_GPIO_PIN(pin) ((uint16_t) (1 << ( pin )))
>
> /**
> * @brief Get EXTI Line from pin number 0-15
> * @param pin is the pin number from 0-15
> */
> -#define STM32F4_GET_LL_EXTI_LINE(pin) (LL_EXTI_LINE_x[( pin )])
> +#define STM32F4_GET_LL_EXTI_LINE(pin) (0x1UL << ( pin ))
>
> /**
> * @brief Get EXTI IRQ number from pin 0-15
> @@ -229,14 +200,6 @@ static stm32f4_interrupt isr_table[16];
>
> void exti_handler(void *arg);
>
> -/************* Helpers implementation ********************/
> -
> -static stm32f4_gpio *get_gpio_from_base(
> - rtems_gpio *base
> -)
> -{
> - return RTEMS_CONTAINER_OF(base, stm32f4_gpio, base);
> -}
>
> /********** STM32F4 GPIO API functions ************/
>
> @@ -523,7 +486,8 @@ rtems_status_code stm32f4_gpio_remove_interrupt(
> rtems_status_code sc = rtems_interrupt_handler_remove(
> STM32F4_GET_EXTI_IRQn(gpio->pin),
> exti_handler,
> - &isr_table[gpio->pin].arg);
> + &isr_table[gpio->pin].arg
> + );
> if (sc == RTEMS_SUCCESSFUL) {
> isr_table[gpio->pin] = (stm32f4_interrupt){0};
> }
> @@ -554,9 +518,12 @@ rtems_status_code stm32f4_gpio_write(
> )
> {
> stm32f4_gpio *gpio = get_gpio_from_base(base);
> - uint32_t pin_mask = STM32F4_GET_HAL_GPIO_PIN(gpio->pin);
>
> - HAL_GPIO_WritePin(gpio->port, pin_mask, value);
> + if (value)
> + LL_GPIO_SetOutputPin(gpio->port,
> STM32F4_GET_HAL_GPIO_PIN(gpio->pin));
> + else
> + LL_GPIO_ResetOutputPin(gpio->port,
> STM32F4_GET_HAL_GPIO_PIN(gpio->pin));
> +
> return RTEMS_SUCCESSFUL;
> }
>
> @@ -566,9 +533,11 @@ rtems_status_code stm32f4_gpio_read(
> )
> {
> stm32f4_gpio *gpio = get_gpio_from_base(base);
> - uint32_t pin_mask = STM32F4_GET_HAL_GPIO_PIN(gpio->pin);
>
> - *value = HAL_GPIO_ReadPin(gpio->port, pin_mask);
> + *value = LL_GPIO_IsInputPinSet(
> + gpio->port,
> + STM32F4_GET_HAL_GPIO_PIN(gpio->pin)
> + );
> return RTEMS_SUCCESSFUL;
> }
>
> @@ -577,9 +546,11 @@ rtems_status_code stm32f4_gpio_toggle(
> )
> {
> stm32f4_gpio *gpio = get_gpio_from_base(base);
> - uint32_t pin_mask = STM32F4_GET_HAL_GPIO_PIN(gpio->pin);
>
> - HAL_GPIO_TogglePin(gpio->port, pin_mask);
> + LL_GPIO_TogglePin(
> + gpio->port,
> + STM32F4_GET_HAL_GPIO_PIN(gpio->pin)
> + );
> return RTEMS_SUCCESSFUL;
> }
>
> --
> 2.36.1
>
> _______________________________________________
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
