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

bcm_driver: revert packed attribute for scan structure #10715

Merged
merged 1 commit into from Sep 21, 2023

Conversation

zhhyu7
Copy link
Contributor

@zhhyu7 zhhyu7 commented Sep 19, 2023

Summary

These structures need to be aligned with the firmware, packed_struct will cause the scan information from Wi-Fi firmware not to be parsed by broadcom Wi-Fi driver. Therefore, we need to remove the packed_struct of the structure associated with the scan information.

Impact

Testing

Cortex-M55 and bcm43xxx chipset

@acassis
Copy link
Contributor

acassis commented Sep 19, 2023

Please include more details in the commit log message. I think the original patch you submitted was trying to packed the struct to optimize memory usage and end up creating other issues. It is important to describe better for other people understand why you already tried and why it failed.

Copy link
Contributor

@acassis acassis left a comment

Choose a reason for hiding this comment

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

Please include a more informative log message (similar to your Summary)

These structures need to be aligned with the firmware, packed_struct will cause the scan information from Wi-Fi firmware not to be parsed by broadcom Wi-Fi driver. Therefore, we need to remove the packed_struct of the structure associated with the scan information.

Signed-off-by: zhanghongyu <zhanghongyu@xiaomi.com>
@zhhyu7
Copy link
Contributor Author

zhhyu7 commented Sep 21, 2023

Please include more details in the commit log message. I think the original patch you submitted was trying to packed the struct to optimize memory usage and end up creating other issues. It is important to describe better for other people understand why you already tried and why it failed.

Done.

@xiaoxiang781216 xiaoxiang781216 merged commit 9267dbc into apache:master Sep 21, 2023
26 checks passed
@pkarashchenko
Copy link
Contributor

Shouldn't this be handled by explicit struct padding or insertion of reserved byte members? I mean that packing option may change depending on arch and compiler, so this case is most likely tuned for 32bit arch with gcc/llvm.

@pkarashchenko
Copy link
Contributor

Ideally the packed and non-packed structures should be the same, so we do not rely on compiler settings. There is also a flexibility in usage of explicit pointer cast from byte array to structure type and packed structures will avoid unaligned pointer usage.

@jerpelea jerpelea added this to To-Add in Release Notes - 12.3.0 Sep 26, 2023
@jerpelea jerpelea moved this from To-Add to net in Release Notes - 12.3.0 Sep 27, 2023
@jerpelea jerpelea moved this from net to done in Release Notes - 12.3.0 Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants