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

spi_slave_api.c: Allow to use GPIO 32 and 33 for interface signals. #248

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

robert-hh
Copy link

Using consistently the IDF API instead of direct port access. There is a small performace drop compared to direct port access for setting the pin, but that can be accepted in favoer of consistence.

GPIO33 for instance is used by Adafruit and NINA modules for the handshake signal, and these modules cannot be rewired.

@CLAassistant
Copy link

CLAassistant commented Jul 24, 2023

CLA assistant check
All committers have signed the CLA.

@mantriyogesh
Copy link
Collaborator

There are some issues in the PR. can you please look into?

@robert-hh
Copy link
Author

As far as I see, the conflict is just a more literal one, not finally affecting the code's operation. It seems that some changes have been made to the master since I made the PR. I'll resolve it.

Using consistently the IDF API instead of direct port access. There
is a small performace drop compared to direct port access for setting
the pin, but that can be accepted in favoer of consistence.

GPIO33 for instance is used by Adafruit and NINA modules for the
handshake signal, and these modules cannot be rewired.

Signed-off-by: robert-hh <robert@hammelrath.com>
@robert-hh
Copy link
Author

The PR is updated now. It has no conflict with the master branch and a test build works with Adafruit Airlft modules.

@mantriyogesh
Copy link
Collaborator

I understand the rework pain. But this MR is important to handle GPIOs > 32..

@mantriyogesh
Copy link
Collaborator

Change looks good. We will pull this in next week.

@mantriyogesh
Copy link
Collaborator

mantriyogesh commented Jul 7, 2024

Thank you @robert-hh @JAndrassy for all your efforts and time ..

@robert-hh
Copy link
Author

@JAndrassy The actual PR does the same with the defines a few lines before defining the masks. I know that this is a kind of additional step. but GPIO_HS amd GPIO_DR are also used another place in the code.

#define GPIO_HS                    CONFIG_ESP_SPI_GPIO_HANDSHAKE
#define GPIO_DR                    CONFIG_ESP_SPI_GPIO_DATA_READY

#define GPIO_MASK_DATA_READY (1ull << GPIO_DR)
#define GPIO_MASK_HANDSHAKE (1ull << GPIO_HS)

@robert-hh
Copy link
Author

Change looks good. We will pull this in next week.

@mantriyogesh Thanks.

@JAndrassy
Copy link

@robert-hh yes sorry. I have my changes in the 0.0.5 tag sources

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