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

drivers/net: update format specifier on lan9250 driver #15915

Merged
merged 1 commit into from
Mar 5, 2025

Conversation

fdcavalcanti
Copy link
Contributor

Summary

Fix print statement for uint8 and uint16 types.
Required due to efforts on fixing #15755 .

I had recently sent a PR on this same driver but missed a couple of lines.

Error example:

In file included from net/lan9250.c:34:
net/lan9250.c: In function 'lan9250_wait_ready':
net/lan9250.c:620:12: warning: format '%x' expects argument of type 'unsigned int', but argument 4 has type 'uint32_t' {aka 'long unsigned int'} [-Wformat=]
  620 |       nerr("ERROR: wait register:0x%02x, mask:0x%08x, expected:0x%08x\n",
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  621 |             address, mask, expected);
      |                      ~~~~
      |                      |
      |                      uint32_t {aka long unsigned int}
net/lan9250.c:620:52: note: format string is defined here
  620 |       nerr("ERROR: wait register:0x%02x, mask:0x%08x, expected:0x%08x\n",
      |                                                 ~~~^
      |                                                    |
      |                                                    unsigned int
      |                                                 %08lx

Impact

Testing

This test simply builds the binary and checks for compilation errors.
Unfortunately I don't have this device to test (requires external LAN9250). If someone has, please speak up!

Building

  • ./tools/configure.sh esp32s3-devkit:eth_lan9250
  • make

Results

No build errors.

@github-actions github-actions bot added Area: Networking Effects networking subsystem Size: XS The size of the change in this PR is very small labels Feb 27, 2025
@fdcavalcanti
Copy link
Contributor Author

Can we go forward with this?

@acassis acassis requested a review from lupyuen February 28, 2025 19:24
@acassis
Copy link
Contributor

acassis commented Feb 28, 2025

Unfortunately, we need to wait until more people vote, I wish more people were actually interested in becoming reviewers. By the way, are you interested in becoming a reviewer?

@lupyuen lupyuen requested a review from cederom February 28, 2025 23:47
Copy link
Contributor

@cederom cederom left a comment

Choose a reason for hiding this comment

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

Thanks @fdcavalcanti :-)

  • The uint16_t address should use 0x%04"PRIx16 as 8-bit data is represented by two ascii hex while 16-bit by four ascii hex right?
  • If we use 0x%02"PRIx8 for uint16_t type then half of the data is cut.
  • Personally I prefer PRIX16 in order to get upper case ascii hex but the code uses already lowercase so its okay to leave its lowercase :-P

@cederom
Copy link
Contributor

cederom commented Mar 1, 2025

@acassis: Unfortunately, we need to wait until more people vote, I wish more people were actually interested in becoming reviewers. By the way, are you interested in becoming a reviewer?

Well, most of us are volunteers working in a so called free moments so its not always possible to respond rapidly as we would wish, help is needed.. for instance its 0308AM Friday/Saturday night here and I just did my first Saturday coffee "today" :D :D

@fdcavalcanti
Copy link
Contributor Author

@acassis: Unfortunately, we need to wait until more people vote, I wish more people were actually interested in becoming reviewers. By the way, are you interested in becoming a reviewer?

Well, most of us are volunteers working in a so called free moments so its not always possible to respond rapidly as we would wish, help is needed.. for instance its 0308AM Friday/Saturday night here and I just did my first Saturday coffee "today" :D :D

Sorry about the delay. Carnival week here in Brazil :)
Sure, I would like to help on reviews @acassis.

Fix print statement for uint8 and uint16 types.

Signed-off-by: Filipe Cavalcanti <[email protected]>
@fdcavalcanti fdcavalcanti force-pushed the feature/fix-lan9250-type branch from 83bcb69 to f4f6108 Compare March 5, 2025 11:52
Copy link
Contributor

@cederom cederom left a comment

Choose a reason for hiding this comment

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

Thank you @fdcavalcanti :-)

@cederom
Copy link
Contributor

cederom commented Mar 5, 2025

CI failed, seems random, restarted. lets wait for the all tests to complete before merge :-)

@cederom
Copy link
Contributor

cederom commented Mar 5, 2025

Allright CI passed :-)

@cederom cederom merged commit 5e52e75 into apache:master Mar 5, 2025
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Networking Effects networking subsystem Size: XS The size of the change in this PR is very small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants