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

boards/samv7/hsmci: add option to invert card detection pin #8577

Merged
merged 1 commit into from
Feb 19, 2023

Conversation

Cynerd
Copy link
Contributor

@Cynerd Cynerd commented Feb 18, 2023

The original code hard-coded card detection to the low when card is inserted and high when not. This might not be true on every board because it depends on the slot and wiring used. The second reason is because it is also possible to detect card with D3 pin pull-up when the slot does not provide dedicated card detection switch.

This introduces new argument to the sam_hsmci_initialize to allow invert of card detection pin. It also applies this invert to existing boards as that was the state up to this point.

The original code hard-coded card detection to the low when card is
inserted and high when not. This might not be true on every board
because it depends on the slot and wiring used. The second reason is
because it is also possible to detect card with D3 pin pull-up when the
slot does not provide dedicated card detection switch.

This introduces new argument to the sam_hsmci_initialize to allow
invert of card detection pin. It also applies this invert to existing
boards as that was the state up to this point.
@xiaoxiang781216 xiaoxiang781216 merged commit 74790c8 into apache:master Feb 19, 2023
@@ -59,7 +59,7 @@ extern "C"
****************************************************************************/

int sam_hsmci_initialize(int slotno, int minor, gpio_pinset_t cdcfg,
int cdirq);
int cdirq, bool cdinvert);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm just wondering of this solution as adding define BOARD_HSMCI_CD_STATE to board.h seems to me a chipper solution.
@Cynerd do you have any objections if I will rework the code and remove parameter + new data in favour of CD "inserted" level define in board.h?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am ok with that.

Honestly, that was my original version and idea. I ended up on this because the original author probably expected support for multiple SD card slots; that was my explanation for passing card detection pin. In such case, the assumption, when using macro, is that everery slot on the board is wired the same way. I do not have nothing against that assumption. I also not see a board with more than one slot in my vicinity. I only implemented it this way to keep it consistent with already established approach of passing card detection configuration.

In short, go ahead.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah. If the different slots have different CD level routing on PCB (I can't imagine why that might happen, but still) then define in board.h is definitely a limitation. I think that the issue appeared when I moved code to the common folder, so the CD level was moved out from board specific.
Seems that the current approach is more flexible, let's stick with that for now.

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.

3 participants