Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GPIO x86 Implementation #6

Merged
merged 5 commits into from
Nov 24, 2024
Merged

GPIO x86 Implementation #6

merged 5 commits into from
Nov 24, 2024

Conversation

mswq
Copy link
Contributor

@mswq mswq commented Nov 12, 2024

No description provided.

Copy link
Member

@Akashem06 Akashem06 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks good, small changes.

Also please update the x86/gpio_mcu.h. I want to reduce the number of ports available in x86 because there are very few times we access port H.

typedef enum {
GPIO_PORT_A = 0,
GPIO_PORT_B,
GPIO_PORT_C,
GPIO_PORT_H,
NUM_GPIO_PORTS,
} GpioPort;

Remove GPIO_PORT_H. Thanks!

@@ -17,28 +17,130 @@
#include "gpio_mcu.h"
#include "status.h"

static GpioMode s_gpio_pin_modes[GPIO_TOTAL_PINS];
static uint8_t s_gpio_pin_state[GPIO_TOTAL_PINS];
static GpioMode s_gpio_alternative_function_modes[GPIO_TOTAL_PINS];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to s_gpio_alt_functions?

return STATUS_CODE_UNIMPLEMENTED;
for (uint32_t k = 0; k < GPIO_TOTAL_PINS; ++k) {
s_gpio_pin_state[k] = GPIO_STATE_LOW;
s_gpio_alternative_function_modes[k] = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

= GPIO_ALT_NONE

The 0 works aswell, its just using the define makes the code more readable


s_gpio_pin_modes[index] = pin_mode;
s_gpio_pin_state[index] = init_state;
s_gpio_alternative_function_modes[index] = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

= GPIO_ALT_NONE

@@ -17,28 +17,130 @@
#include "gpio_mcu.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the header seen in other files:

/************************************************************************************************

  • gpio.c
  • GPIO Library Source Code
  • Created: 2024-11-02
  • Midnight Sun Team #24 - MSXVI
    ************************************************************************************************/

And sort the #includes into the correct sections:

/* Standard library headers */

/* Inter-component Headers */

/* Intra-component Headers */

return status_code(STATUS_CODE_INVALID_ARGS);
}

if (s_gpio_alternative_function_modes[index] != 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't need this check, alternate function pins can be driven high and low. Typically it is not done by our code but still, shouldn't be here


uint32_t index = address -> port * (uint32_t)GPIO_PINS_PER_PORT + address -> pin;

if (s_gpio_alternative_function_modes[index] != 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, alternative function pins can be driven high/low

@Bafran Bafran changed the title a GPIO x86 Implementation Nov 14, 2024
@Bafran
Copy link
Collaborator

Bafran commented Nov 14, 2024

Make sure your PR has a proper name, changed to GPIO x86 Implementation

mzqan and others added 3 commits November 23, 2024 21:04
* created mpu header and source file

* Updated mpu.h and mpu.c files
Copy link
Member

@Akashem06 Akashem06 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Akashem06 Akashem06 merged commit 57991e1 into main Nov 24, 2024
1 check passed
@Akashem06 Akashem06 deleted the gpiox86 branch November 24, 2024 02:13
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.

4 participants