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

Allow any gpio pin to be used (not just the first 32) #117

Open
wants to merge 6 commits into
base: sdk-2.0.0
Choose a base branch
from

Conversation

georgeharker
Copy link

Previous implementation disallowed using higher pin numbers for srst etc, this removes that restriction

replaces #113

Just used gpio number to decode register locations
@lurch lurch changed the title Allow any gpis pin to be used (not just the first 32) Allow any gpio pin to be used (not just the first 32) Oct 3, 2024
@@ -93,9 +97,9 @@ static void set_gpio_value(const struct adapter_gpio_config *gpio_config, int va
switch (gpio_config->drive) {
case ADAPTER_GPIO_DRIVE_MODE_PUSH_PULL:
if (value)
GPIO_SET = 1 << gpio_config->gpio_num;
GPIO_SET(gpio_config->gpio_num) = 1 << (gpio_config->gpio_num & 0x1f);
Copy link

Choose a reason for hiding this comment

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

IMHO it looks a bit weird seeing gpio_config->gpio_num repeated on both sides of the = sign? But perhaps "fixing" that would be too much of a departure from the way the existing code was working? 🤷

Copy link
Author

Choose a reason for hiding this comment

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

I agree - could rewrite entirely if you'd like and make set an entire macro?

Copy link

Choose a reason for hiding this comment

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

Not really my call to make. What does @P33M think?

Copy link
Author

Choose a reason for hiding this comment

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

@P33M any thoughts?

@lurch
Copy link

lurch commented Oct 3, 2024

Is the motivation for this PR to be able to use this with the additional GPIO pins that are exposed on the Compute Modules?

@lurch
Copy link

lurch commented Oct 3, 2024

I've just noticed that this PR introduces a mix of tabs vs. spaces for indentation. IMHO it's best to be consistent (so stick to tabs, as that's what the original code was using).

@georgeharker
Copy link
Author

Is the motivation for this PR to be able to use this with the additional GPIO pins that are exposed on the Compute Modules?

Yes, I have a board which is pin constrained and I needed to be able to use GPIO 44 as the reset pin... it wasn't initially clear that the request to do so was silently ignored. In theory any pin should be usable... hence the PR

@georgeharker
Copy link
Author

I've just noticed that this PR introduces a mix of tabs vs. spaces for indentation. IMHO it's best to be consistent (so stick to tabs, as that's what the original code was using).

Thanks, fixed. I usually have my editor set up to auto convert, missed that in the diff.

@georgeharker
Copy link
Author

I've just noticed that this PR introduces a mix of tabs vs. spaces for indentation. IMHO it's best to be consistent (so stick to tabs, as that's what the original code was using).

this should now be fixed

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.

2 participants