Bug in LL library for stm32f1xx

[amomum]

Posted on September 27, 2017 at 19:03

I want to turn on the led attached to PC.9. I do this:

    LL_GPIO_InitTypeDef initStruct;

    LL_GPIO_StructInit( &initStruct );

    initStruct.Mode = LL_GPIO_MODE_OUTPUT_10MHz;

    initStruct.OutputType = LL_GPIO_OUTPUT_PUSHPULL;

    initStruct.Speed = LL_GPIO_SPEED_FREQ_HIGH;

    initStruct.Pin = LL_GPIO_PIN_9; // that equals 0x04020002

    LL_GPIO_Init( GPIOC, &initStruct )

This code will actually init PC.2 (not PC.9 as it's supposed to) to output push-pull.

Why? LL_GPIO_Init does this:

    pinpos = POSITION_VAL(GPIO_InitStruct->Pin);

where

    &sharpdefine POSITION_VAL(VAL)     (__CLZ(__RBIT(VAL)))

so pinpos will be equal to 

(__CLZ(__RBIT(

0x04020002

))) which is equal to 1.

Then current pin will be calculated like this:

    currentpin = (GPIO_InitStruct->Pin) & (0x00000101U << pinpos);

so currentpint will be equal to 2.

And then

    LL_GPIO_SetPinMode(GPIOx, currentpin, GPIO_InitStruct->Mode);

will be called, despite the fact that LL_GPIO_SetPinMode should only accept parameters like LL_GPIO_PIN_x.

Reasons:

•   all inline functions in header files do not check their arguments with assert_param, so this error was not caught early
•   lack of proper (i.e. automatic) testing?
•   lack of public bugtracker

I have already posted this bug here but it got mixed up with my own ignorance and stupidity so I haven't got a proper reaction. I believe it's time to post it again.

#hal-ll #ll-bug #software-error #bug #ll #stm32f1

[loeffel2]

Posted on February 19, 2018 at 09:44

If we decide to fix the issue. I well share it.

But first we have to figure out what the best solution would be for us. It seems that the LL for the STM32F1xx devices is a unstable release if a fundamental function like the pin initialization is not working.  

Best regards Raphael

[loeffel2]

Posted on February 19, 2018 at 11:05

Simon Brouwer already posted a fix of the issue in his first response. Thanks for the work and sorry for overriding the post. 

Best regards Raphael

[Bogdan Golab]

Posted on February 19, 2018 at 11:32

Thank you for letting me know. I just missed it. Probably because I did quick hack for myself.

[Imen.D]

Posted on February 19, 2018 at 12:01

Hello,

This issue is confirmed for LL Gpio pins greater than 8 initialization with LL-Init() function and this will be fixed in the next release.

We propose thisfix for LL_GPIO_Init function in the ll_gpio.c driver:

ErrorStatus LL_GPIO_Init(GPIO_TypeDef *GPIOx, LL_GPIO_InitTypeDef *GPIO_InitStruct)
{
 uint32_t pinmask = 0x00000000U;
 uint32_t pinpos = 0x00000000U;
 uint32_t currentpin = 0x00000000U;
 
 /* Check the parameters */
 assert_param(IS_GPIO_ALL_INSTANCE(GPIOx));
 assert_param(IS_LL_GPIO_PIN(GPIO_InitStruct->Pin));
 assert_param(IS_LL_GPIO_MODE(GPIO_InitStruct->Mode));
 assert_param(IS_LL_GPIO_PULL(GPIO_InitStruct->Pull));
 
 /* ------------------------- Configure the port pins ---------------- */
 /* Initialize pinpos on first pin set */
 
 pinmask = (GPIO_InitStruct->Pin & 0x00FFFF00U) >> 8 ;
 pinpos = POSITION_VAL(pinmask);
/* Configure the port pins */
 while ((pinmask >> pinpos) != 0U)
 {
 /* skip if bit is not set */
 if ((pinmask & (1U << pinpos)) == 0U)
 {
 pinpos++;
 }
/* Get current io position */
 if(pinpos <8 )
 {
 currentpin = (0x00000101U << pinpos);
 }
 else
 {
 currentpin = ((0x00010001U << (pinpos-8)) | 0x04000000U);
 }
 
 if (currentpin)
 {
 /* Pin Mode configuration */
 LL_GPIO_SetPinMode(GPIOx, currentpin, GPIO_InitStruct->Mode);
 
 /* Pull-up Pull down resistor configuration*/
 LL_GPIO_SetPinPull(GPIOx, currentpin, GPIO_InitStruct->Pull);
 
 if ((GPIO_InitStruct->Mode == LL_GPIO_MODE_OUTPUT) || (GPIO_InitStruct->Mode == LL_GPIO_MODE_ALTERNATE))
 {
 /* Check Output mode parameters */
 assert_param(IS_LL_GPIO_OUTPUT_TYPE(GPIO_InitStruct->OutputType));
/* Output mode configuration*/
 LL_GPIO_SetPinOutputType(GPIOx, currentpin, GPIO_InitStruct->OutputType);
/* Speed mode configuration */
 LL_GPIO_SetPinSpeed(GPIOx, currentpin, GPIO_InitStruct->Speed);
 }
}
 pinpos++;
 }
 return (SUCCESS);
}�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?

Hope this is helpful for you.

Kind Regards,

Imen.

[Simon Brouwer]

Posted on February 19, 2018 at 21:38

Hi Imen, I think if you leave out the 'continue; ', some pins may be affected that correspond to a 0 bit in the pinmask (after skipping the first 0 bit, the next pin will be configured even if its bit in the pinmask is 0)

[Federico Zuccardi Merli]

Posted on March 09, 2018 at 15:23

Hi Imen,

ST should pay more attention when proposing code as a workaround.

If one wants to gloss over the pre-existing overcomplicated and bizarre definition of the pin constants for the F1, justified only by a small possible saving in scarcely used initialization routines, the proposed code is both redundant, as

• it pointlessly initialize variables that will always be set
• it checks currrentpin for inequality to 0, guaranteed by the statement right above the check

and wrong, as Simon Brouwer already pointed out.

Is there a publicly accessible bug tracker for the LL_ library (and the HAL)?

[Imen.D]

Posted on March 12, 2018 at 11:02

Hello

simon.010

,

Your proposal fix is raised internally to developer team for check.

Thanks andBest Regards,

Imen

[Bogdan Golab]

Posted on March 16, 2018 at 19:20

Just tested a STM32VLDIscovery board and the issue still exists in CubeMX 4.25.0 with the v1.6.0 HAL/LL drivers.

[Bogdan Golab · Accepted solution]

Posted on March 19, 2018 at 17:08

Just tested STM32CubeF1 Firmware patch Package V1.6.1 - GPIO works fine.

[Imen.D]

Posted on March 22, 2018 at 09:53

Hello,

We would inform you that the new patch of STM32CubeF1 Firmware package V1.6.1 is released to fix this issue.

Best Regards

Imen