openhwgroup / core-v-mcu

This is the CORE-V MCU project, hosting CORE-V's embedded-class cores.
https://docs.openhwgroup.org/projects/core-v-mcu
Other
158 stars 50 forks source link

[BUG] Unable to read switch status it always return 1 #304

Closed WazaAbdulkadir closed 1 year ago

WazaAbdulkadir commented 1 year ago

Is there an existing core-v-mcu bug for this?

Bug Description

I am using core-v-sdk to generate a bare-metal application on nexsys board.

My application is reading switch value and light the corresponding led.

For this I remap the I/Os from the constraint file as:

set_property -dict { PACKAGE_PIN V16 IOSTANDARD LVCMOS33 } [get_ports { xilinx_io[21]}]; #IO_L16N_T2_A15_D31_14 Sch=led[8]

set_property -dict { PACKAGE_PIN T8 IOSTANDARD LVCMOS18 } [get_ports { xilinx_io[29] }]; #IO_L24N_T3_34 Sch=sw[8] After this I programmed my device. ( I have tested its switches and leds. In normal conditions they work properly.)

I have developed two different software for my application at first I used hal_read_gpio_status_raw() function. Here is my code:

int main(void)
{
    prvSetupHardware();

     // SET LED AS OUTPUT

     hal_setpinmux(21,2);
     hal_set_gpio_mode(14,1);

     // SET SWITCH AS INPUT

     hal_setpinmux(29,2);
     //hal_setpullup(29,1);
     hal_set_gpio_mode(22,0);

     uint8_t  switch1        = 0;
     uint32_t register_value = 0;

     hal_write_gpio(14,0);
     while(1){

        hal_read_gpio_status_raw(22,&register_value);

        switch1 = (register_value >> 12) & 0x01;

        if (switch1==1) {
            hal_write_gpio(14,1);
        }

        else {
            hal_write_gpio(14,0);

        }

    }
}

From debugging I get

Binary:1000000010110

Binary value is what register_value holds, this shows program accepts gpio_22 as an input. 13th bit shows input value and as in this case hal_read_gpio_status_raw() function always returns 1, regardless of the switch position.

I have also tried to create this application with using hal_read_gpio_status() function. This function also always return input value as 1.

Code is below:

 hal_setpinmux(21,2);
 hal_set_gpio_mode(14,1);

 // SET SWITCHES AS INPUT

  hal_setpinmux(29,2);
  gpio_hal_typedef switch1;
  switch1.number = 22;
  switch1.mode   = 0 ;

  hal_write_gpio(14,0);
  while(1){

      hal_read_gpio_status(&switch1);

     if (switch1.in_val = 1) {
         hal_write_gpio(14,1);
     }
     else {
         hal_write_gpio(14,0);
    }

What could be the problem?

MikeOpenHWGroup commented 1 year ago

Hi @WazaAbdulkadir, this issue seems to be a duplicate of core-v-mcu-cli-test #48. It would be most helpful if you could attempt to determine whether the issue is in the MCU itself or CLI-Test.

WazaAbdulkadir commented 1 year ago

Hi. This issue is in the MCU. It is better to argue about this here.

MikeOpenHWGroup commented 1 year ago

OK, in that case, please close core-v-mcu-cli-test #48.

@gmartin102, do you have any insight on this?

gmartin102 commented 1 year ago

This problem is due to the design putting weak pull-ups on all outputs. The switch on the nexys board connects to ground or VCC via a 10K series resistor and the weak pull-up prevents the pin from going fully low. I have verified that your case behaves as you specified, but if I disable the weak pull, the gpio behaves as expected.

WazaAbdulkadir commented 1 year ago

I have tried this before. I used hall_setpullup() function to disable pull-up. Is this what you mean? It wasn't solved my problem. After your comment I tried it again, still there is no progress. Am i missing something?

WazaAbdulkadir commented 1 year ago

After commenting (* PULLUP = "YES" *) line in the pad_functional_xilinx.sv source file gpio behaved as expected.

MikeOpenHWGroup commented 1 year ago

Hi @WazaAbdulkadir, can you create a PR for this?

WazaAbdulkadir commented 1 year ago

Hi Mike. I just comment out a line. I think a better approach could be creating a pad_functional_np (no-pull) function and using it on core_v_mcu_nexsys.v. It seems like internal pull-up resistors will always set input pins to high.

gmartin102 commented 1 year ago

The correct way to address this is to create a pad_functional_np module, but the place to instantiate it is not in core_v_mcu_nexys.v but in utils/ioscript.py. the core_v_nexys.v is generated file from the ioscript.py using the nexys-pin-table.csv ... but once a user starts modifying the base RTL (reassigning pins, etc) it is really up to the user to understand how the bitstream is created. Modifying generated files by hand is perfectly acceptable for an individual user, but I wouldn't suggest pushing those changes to the repo. I'm not sure whats to be gained by pushing a small no pull-up module change unless someone incorporates the ability to add pu/pd/np to the ioscript on a per io basis.

MikeOpenHWGroup commented 1 year ago

Ah, I had not taken the time to look and see that we are talking about generated files. I agree with @gmartin102. @WazaAbdulkadir, if that works for you please close this issue. Thanks both!