Issue with STM32F1 pull up pin

[yashrk]

Hey guys, I am trying to learn bare metal programming and I was trying to get a pin B3 to be set as pull up, with following code. I am trying to check if the pin is pulled up via a multimeter and yes it is not pulled up. I have one pin C13 blinking an LED which is working fine.

I have written simple wrappers to make my life easy.

Code: [Select]

#include "stm32f10x.h"
#include "_HAL_GPIO.h"


GPIO_TYPE ledGPIO;
GPIO_TYPE inputPin;

int main(void){
// Configuring GPIO

ledGPIO.port = PORTC;
ledGPIO.pin = 13;
ledGPIO.mode = OUTPUT_MODE;
ledGPIO.mode_type = OUTPUT_GEN_PURPOSE;
ledGPIO.speed = SPEED_50MHZ;

gpio_init(ledGPIO);

inputPin.port = PORTB;
inputPin.pin = 3;
inputPin.mode = INPUT_MODE;
inputPin.mode_type = INPUT_PU_PD;

gpio_init(inputPin);
inputPin.port -> ODR |= (1 << inputPin.pin); // Setting pull up


while(1) {

gpio_write(ledGPIO.port, ledGPIO.pin, LOW);
for ( int i = 0; i <= 5000000; i++);
gpio_write(ledGPIO.port, ledGPIO.pin, HIGH);
for ( int i = 0; i <= 5000000; i++);

}

}




[wek]

What hardware, a "known good" like Nucleo ro Disco, or your own? In the latter case, is there anything connected to that pin? Does setting PB3 to GPIO Output and "blinking" it does produce the expected changes on the output?

Read out and check/post content of GPIO registers.

JW

[yashrk]

What hardware, a "known good" like Nucleo ro Disco, or your own? In the latter case, is there anything connected to that pin?

This is a Bluepill, but you gave me a good idea that I can try running it using CubeIDE and then see if its an hardware problem or a software problem.

[yashrk]

Yeah programming using CubeIDE also doesn't work 

[pcprogrammer]

The code you posted is missing a lot of information.

You state to have written wrappers to make life easy, but not showing that code makes it harder for us to say what could be wrong.

What I don't get is why you made a structure for holding the information of a GPIO and then not pass that struct to your gpio_write function, but that is not why the code is failing of course.

Another question is what the system setup is. There is no clock configuration, no peripheral enable, etc shown in your code.  Clearly port C clock is enabled when you see the LED blink, but what about port B clock?

I would say, for bare metal stay clear from any ST software crap.

Here is an example project of mine. Fully bare metal.

[DavidAlfa]

Are you enabling the GPIO clocks before configuring them?

Code: [Select]

RCC->AHB1ENR |= 0x6;    // Enable GPIO B and C


[wek]

Read out and check/post content of GPIO registers.

JW

[yashrk]

The code you posted is missing a lot of information.

You state to have written wrappers to make life easy, but not showing that code makes it harder for us to say what could be wrong.

Sorry I was up at night and frustrated, I have attached them now. Thanx!

What I don't get is why you made a structure for holding the information of a GPIO and then not pass that struct to your gpio_write function, but that is not why the code is failing of course.

I am following this Youtube tutorial so I wanted to keep mostly everything the same.

If you have better options to learn video or in written format please suggest, I am open to buy new hardware as well. Only goal of mine is to be capable enough to work on any ARM controller without using HAL type libraries.

[pcprogrammer]

I'm to tired at the moment to concentrate on the source you posted, but it looks unnecessarily complex.

Code: [Select]

//----------------------------------------------------------------------------------------------------------------------------------

//Simple function for setup of an IO pin
void InitIOPin(GPIO_TypeDef *port, uint32_t pin, uint32_t mode, uint32_t conf)
{
  //Mix the mode and configuration for single instruction usage
  uint32_t data = mode | (conf << 2);

  //Create a base pointer for either the lower or the higher control register
  __IO uint32_t *reg;

  //See if the lower control register or the higher control register needs to be used
  if(pin < 8)
  {
    //Low control register used for first 8 pins
    reg = &port->CRL;
  }
  else
  {
    //Force pin into 8 pins per register range
    pin -= 8;

    //High control register used for upper 8 pins
    reg = &port->CRH;
  }

  //4 control bits used per pin
  pin *= 4;

  //Reset bits first and set new mode and configuration.
  *reg &= ~(0x0F << pin);
  *reg |=  (data << pin);
}

//----------------------------------------------------------------------------------------------------------------------------------

uint32_t readIOPin(GPIO_TypeDef *port, uint32_t pin)
{
  //Convert pin number to a bit mask
  register uint32_t mask = (1 << pin);
 
  //Get the masked status of the pin and match it with the mask to return either 0 or 1
  return((port->IDR & mask) == mask);
}

//----------------------------------------------------------------------------------------------------------------------------------

void setIOPin(GPIO_TypeDef *port, uint32_t pin)
{
  //Convert pin number to a set bit mask and set the output
  port->ODR |= (1 << pin);
}

//----------------------------------------------------------------------------------------------------------------------------------

void clearIOPin(GPIO_TypeDef *port, uint32_t pin)
{
  //Convert pin number to a clear bit mask and clear the output
  port->ODR &= ~(1 << pin);
}

//----------------------------------------------------------------------------------------------------------------------------------

This code uses the structure for the register map of GPIO and the pin number. Mode and configuration settings are the same values as in your code. Only extra bit found in your code that is handy is the enabling of the peripheral clock for the GPIO port.

I don't have any good suggestions for learning other than reading the datasheets and maybe a good book on programming. I have learned all this stuff long before the internet came along with youtube tutorials. Did it by reading books and studying datasheets. 

[radiolistener]

you're needs to configure clock and enable clock for GPIO port

[yashrk]

you're needs to configure clock and enable clock for GPIO port

I have this function doing it.

Code: [Select]

void gpio_init (GPIO_TYPE gpio_type){
    if(gpio_type.port == PORTA)
        GPIO_CLOCK_ENABLE_PORTA;

    if(gpio_type.port == PORTB)
        GPIO_CLOCK_ENABLE_PORTB;

    if(gpio_type.port == PORTC)
        GPIO_CLOCK_ENABLE_PORTC;

    if(gpio_type.port == PORTD)
        GPIO_CLOCK_ENABLE_PORTD;

    config_pin(gpio_type.port, gpio_type.pin, gpio_type.mode_type);
    config_pin_speed(gpio_type.port, gpio_type.pin, gpio_type.speed, gpio_type.mode);
}

and defines are as follows.

Code: [Select]

//CLOCK ENABLING
#define GPIO_CLOCK_ENABLE_ALT_FUNC  (RCC -> APB2ENR |= ( 1 << 0 ))
#define GPIO_CLOCK_ENABLE_PORTA     (RCC -> APB2ENR |= ( 1 << 2 ))
#define GPIO_CLOCK_ENABLE_PORTB     (RCC -> APB2ENR |= ( 1 << 3 ))
#define GPIO_CLOCK_ENABLE_PORTC     (RCC -> APB2ENR |= ( 1 << 4 ))
#define GPIO_CLOCK_ENABLE_PORTD     (RCC -> APB2ENR |= ( 1 << 5 ))


[pcprogrammer]

Take a good look at the code that sets the mode and configuration bits in the GPIO control registers. Either low or high control register.

It uses two separate functions to set the speed and the configuration but very nontransparent. It might be correct but I would fire the person who made it on the spot.
Setting up an array to obtain a number that can be calculated with two left shifts is absurd. Then doing bit manipulations where and-ing and or-ing just 4 bits is enough. 

Code: [Select]

static uint32_t PINPOS[16] = {
    (0x00),
    (0x04),
    (0x08),
    (0x0C),
    (0x10),
    (0x14),
    (0x18),
    (0x1C),
    (0x00),
    (0x04),
    (0x08),
    (0x0C),
    (0x10),
    (0x14),
    (0x18),
    (0x1C)
};

static void config_pin (GPIO_TypeDef *port, uint32_t pinNumber, uint32_t mode_type){
    if (pinNumber >= 8){ // CONTROL HIGH REGISTER
        switch (mode_type)
        {
        case OUTPUT_GEN_PURPOSE | INPUT_ANALOG:
            port -> CRH &= ~(( 1 << CNF_POS_BIT1 ) | ( 1 << CNF_POS_BIT2 ));
            break;

        case OUTPUT_OD | INPUT_FLOATING:
            port -> CRH &= ~( 1 << CNF_POS_BIT2);
            port -> CRH |= ~( 1 << CNF_POS_BIT1);
            break;
       
        case OUTPUT_ALT_FUNCTION | INPUT_PU_PD:
            port -> CRH |= OUTPUT_ALT_FUNCTION << (CNF_POS_BIT1);
            break;
       
        case OUTPUT_ALT_FUNCTION_OD:
            port -> CRH |= OUTPUT_ALT_FUNCTION_OD << (CNF_POS_BIT1);
            break;
        }
    }
    else { // CONTROL LOW REGISTER
        switch (mode_type)
        {
        case OUTPUT_GEN_PURPOSE | INPUT_ANALOG:
            port -> CRL &= ~(( 1 << CNF_POS_BIT1 ) | ( 1 << CNF_POS_BIT2 ));
            break;

        case OUTPUT_OD | INPUT_FLOATING:
            port -> CRL &= ~( 1 << CNF_POS_BIT2);
            port -> CRL |= ~( 1 << CNF_POS_BIT1);
            break;
       
        case OUTPUT_ALT_FUNCTION | INPUT_PU_PD:
            port -> CRL |= OUTPUT_ALT_FUNCTION << (CNF_POS_BIT1);
            break;
       
        case OUTPUT_ALT_FUNCTION_OD:
            port -> CRL |= OUTPUT_ALT_FUNCTION_OD << (CNF_POS_BIT1);
            break;
        }
    }
}

static void config_pin_speed (GPIO_TypeDef *port, uint32_t pinNumber, uint32_t pinSpeed, uint32_t mode){
    if (pinNumber >= 8){ // CONTROL HIGH REGISTER
        if (mode == INPUT_MODE)
            port -> CRH &= ~( 1 << (PINPOS[pinNumber]) | 1 << (PINPOS[pinNumber] + 1));
        else
            port -> CRH |= (pinSpeed << (PINPOS[pinNumber]));
    }
    else { // CONTROL LOW REGISTER
        if(mode == INPUT_MODE)
            port -> CRL &= ~( 1 << (PINPOS[pinNumber]) | 1 << (PINPOS[pinNumber] + 1));
        else
            port -> CRL |= (pinSpeed << (PINPOS[pinNumber]));
    }
}


Code: [Select]

// INPUT MODES TYPE
#define INPUT_ANALOG    ((uint32_t) 0x00)
#define INPUT_FLOATING  ((uint32_t) 0x01) // default state at reset
#define INPUT_PU_PD     ((uint32_t) 0x02) // input with pullup or pulldown

// OUTPUT MODE TYPE
#define OUTPUT_GEN_PURPOSE          ((uint32_t) 0x00) // gerneal purpose output
#define OUTPUT_OD                   ((uint32_t) 0x01) // output open drain
#define OUTPUT_ALT_FUNCTION         ((uint32_t) 0x02) // push pull
#define OUTPUT_ALT_FUNCTION_OD      ((uint32_t) 0x03) // open drain

// HIGH BIT POSITION FOR CRH REGISTER CNFYG AND MODE
#define CNF_POS_BIT1    (PINPOS[pinNumber] + 2)
#define CNF_POS_BIT2    (PINPOS[pinNumber] + 3)

With all the macros and the way it is done with the switch statement and the indexed array to change the mode and configuration bits it is needlessly complex.

If you really want to learn bare metal programming, forget that youtube channel and download the ST reference manual for the STM32F103 series and start with writing programs writing directly to the registers. When you understand the working of it try to come up with your own abstraction layer to make it easier for future development.

[DavidAlfa]

I'd use ST LL libraries, they are basically the same..

[GromBeestje]

PB3 is one of the JTAG pins (TDO). You need to release this pin from that role.
Assuming you are running SWD to debug, you could call
LL_GPIO_AF_Remap_SWJ_NOJTAG();
found in stm32f1xx_ll_gpio.h, then the pin will be usable.

Edit:typo

[yashrk]

First, thank you everyone for their comments and reviewing the convoluted code which I shared.

PB3 is one of the JTAG pins (TDO). You need to release this pin from that role.
Assuming you are running SWD to debug, you could call
LL_GPIO_AF_Remap_SWJ_NOJTAG();
found in stm32f1xx_ll_gpio.h, then the pin will be usable.

Tried it didn't work.

If you really want to learn bare metal programming, forget that youtube channel and download the ST reference manual for the STM32F103 series and start with writing programs writing directly to the registers. When you understand the working of it try to come up with your own abstraction layer to make it easier for future development.

Did just that and it worked, thanx! Here is the code BTW if someone stumbles on this thread.

Code: [Select]

#include "stm32f10x.h"

int main(void){
// Configuring GPIO

RCC -> APB2ENR |= ( 1 << 4 ); // Enabling Clock for Port C

// For Pin C13 - LED
GPIOC -> CRH |= ( ( 1 << 20 ) | ( 1 << 21) ); // Set to 50 MHz Output Mode
GPIOC -> CRH &= ( uint32_t ) ~( ( 1 << 22 ) | ( 1 << 23) ); // Set to General Purpose push-pull

// For Pin C14
GPIOC -> CRH &= ( uint32_t ) ~( ( 1 << 24 ) | ( 1 << 25) ); // Set to Input mode 00
// Set Config to 10
GPIOC -> CRH &= ( uint32_t ) ~( 1 << 26 );
GPIOC -> CRH |= ( 1 << 27 );

GPIOC -> ODR |= ( 1 << 14 ); // Set pin pull up


while(1) {

for ( int i = 0; i <= 1000000; i++) ;
GPIOC -> BSRR = ( 1 << 13 ); // Set Pin C13
for ( int i = 0; i <= 5000000; i++) ;
GPIOC -> BSRR = ( 1 << ( 13 + 16 ) ); // Reset Pin C13

}

}

I am planning to use libopencm3 for my future experiments, any comments on that will be helpful.

Also, any suggestion on how to do this bit wise operation better.

Code: [Select]

// Set Config to 10
GPIOC -> CRH &= ( uint32_t ) ~( 1 << 26 );
GPIOC -> CRH |= ( 1 << 27 );


[rhodges]

Here is what I use for STM32F103 ("BLUE_PILL") and STM32F030 ("F030_DEMO").
And here is an example.

Code: [Select]

void local_setup(void)
{
    /* We will use A1 as a general purpose oscilloscope signal and
     * as a trigger for the signal analyzer.
     * A0 is the strobe signal for testing with TM1638 LED+keypad.
     */
    IO_PIN      pin;

    pin.gpio = GPIOA;
    pin.pin_num = 0;
    pin_config(&pin, PIN_OUTPUT | PIN_PUSHPULL | PIN_SPEED_MED);
    GPIOA->ODR = 1;

    pin.pin_num = 1;
    pin_config(&pin, PIN_OUTPUT | PIN_PUSHPULL | PIN_SPEED_MED);
}


[wek]

I am planning to use libopencm3 for my future experiments, any comments on that will be helpful.

Why? (Hope this counts as a comment.)

Also, any suggestion on how to do this bit wise operation better.

Code: [Select]

// Set Config to 10
GPIOC -> CRH &= ( uint32_t ) ~( 1 << 26 );
GPIOC -> CRH |= ( 1 << 27 );


Note, that there is no "best" in microcontroller programming, as appropriateness of any solution is context-dependent.

This particular snippet can be made "better" by reducing number of reads and writes from/to the GPIO register:

Code: [Select]

GPIOC->CRH = (GPIOC->CRH & ~(1 << 26)) | (1 << 27);  // set CNF to 0b10: Input with pull-up / pull-down
and it can be made somewhat "better" to manipulate the whole bitfield at once, which also reveals the whole you are going to write:

Code: [Select]

GPIOC->CRH = (GPIOC->CRH & ~(0b11 << 26)) | (0b10 << 26);  // set CNF to 0b10: Input with pull-up / pull-down
and it can be made yet "better" by emphasizing, that you want to set this field for PC14:

Code: [Select]

GPIOC->CRH = (GPIOC->CRH & ~( 0b11 << (4 * (14 - 8) + 2)) | (0b10 << (4 * (14 - 8) + 2));      // set CNF14 to 0b10: Input with pull-up / pull-down

But, in the vast majority of microcontroller applications, you want to set up all the pins at once, at the startup, so that's one single write to the register; i.e.

Code: [Select]

GPIOC->CRH = 0
           // define for other GPIOC pins 0..15 here, otherwise they will be set to Analog mode (MODE=0b00, CNF=0b00
           | (0b11 << (4 * (13 - 8) + 0))      // set MODE13 to 0b11: 50MHz output
           | (0b00 << (4 * (13 - 8) + 2))      // set CNF13 to 0b00: General purpose output push-pull
           | (0b00 << (4 * (14 - 8) + 0))      // set MODE14 to 0b00: Input
           | (0b10 << (4 * (14 - 8) + 2))      // set CNF14 to 0b10: Input with pull-up / pull-down
       ;

You can make this somewhat more readable by writing macros for CNF and MODE fields.

Others may have different ideas about what's *better*.

JW

PS. Disclaimer: there may be errors in the lines above, did not test.

[DavidAlfa]

That's a mess to maintain and understand.

[wek]

That's a mess to maintain and understand.

Exactly. As I keep saying, "better" is subjective and context dependent.

Here, obviously, the proposed solution is not the "best", and maybe not even "better", for @DavidAlfa.

JW