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

fix: Hitag S logtrace time #2522

Merged
merged 1 commit into from
Sep 25, 2024

Conversation

douniwan5788
Copy link
Contributor

No description provided.

Copy link

You are welcome to add an entry to the CHANGELOG.md as well

armsrc/hitagS.c Outdated
AT91C_BASE_TC0->TC_CCR = AT91C_TC_CLKEN | AT91C_TC_SWTRG;
AT91C_BASE_TC1->TC_CCR = AT91C_TC_CLKEN | AT91C_TC_SWTRG;
AT91C_BASE_TC0->TC_CCR = AT91C_TC_CLKEN;
AT91C_BASE_TC1->TC_CCR = AT91C_TC_CLKEN;
Copy link
Collaborator

Choose a reason for hiding this comment

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

there is a bug mentioned in the errata of the arm chip, use previous declaration.

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 didn't find any Timer Counter info in the errata section. If you are referring to the reset, the three timer counters are reset together using AT91C_BASE_TCB->TCB_BCR = 1

Copy link
Collaborator

Choose a reason for hiding this comment

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

No.

And then I will close this PR.

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'm having a bit of trouble finding the exact TC hardware bug you described. The closest thing I could find was some reset bug. Could it be that the issue you're thinking of is similar to these ones?
5d15891
63a1d80

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, that would be the ones.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great! Then the reset is already handled together using the SYNC signal (AT91C_BASE_TCB->TCB_BCR = 1) which generates a software trigger for each channel simultaneously. I think it's ready to be merged.

Copy link
Collaborator

Choose a reason for hiding this comment

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

No,
Revert back the = AT91C_TC_CLKEN | AT91C_TC_SWTRG parts.

Copy link
Contributor Author

@douniwan5788 douniwan5788 Sep 25, 2024

Choose a reason for hiding this comment

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

But why? The reset is properly handled by the SYNC signal. Is the redundant code for bug-proofing in case someone copy/paste it elsewhere, or just to maintain consistent code style?
Anyway, I will revert it back. Please reopen this PR

armsrc/hitagS.c Outdated
AT91C_BASE_TC1->TC_CCR = AT91C_TC_CLKEN | AT91C_TC_SWTRG;
AT91C_BASE_TC0->TC_CCR = AT91C_TC_CLKEN;
AT91C_BASE_TC1->TC_CCR = AT91C_TC_CLKEN;
AT91C_BASE_TC2->TC_CCR = AT91C_TC_CLKEN;
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as previous comment

@iceman1001 iceman1001 closed this Sep 20, 2024
@iceman1001 iceman1001 reopened this Sep 25, 2024
@iceman1001 iceman1001 merged commit 2dc0946 into RfidResearchGroup:master Sep 25, 2024
12 checks passed
@iceman1001
Copy link
Collaborator

Thank you!

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