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

backport-include: fix linux/acpi_amd_wbrf.h inclusion #1

Closed

Conversation

guidosarducci
Copy link

@guidosarducci guidosarducci commented Jan 2, 2025

Fix building for kernel >= 6.8 by adjusting an incorrect guard usage, otherwise an #include_next header is masked and compilation will fail for net/mac80211/wbrf.c in the mac80211 kernel module.

Fixes: 52cdcaa ("backport-include: backport linux/acpi_amd_wbrf.h")

This patch complements my OpenWrt mac80211 PR #17456.

CC: @nbd168 @hauke @PolynomialDivision @robimarko

Happy New Year!

Fix building for kernel >= 6.8 by adjusting incorrect guard usage,
otherwise an #include_next header is masked and compilation will fail
for net/mac80211/wbrf.c in the mac80211 kernel module.

Fixes: 52cdcaa ("backport-include: backport linux/acpi_amd_wbrf.h")
Signed-off-by: Tony Ambardar <[email protected]>
@namiltd
Copy link

namiltd commented Jan 2, 2025

@guidosarducci
Copy link
Author

I did it differently: namiltd/openwrt@7642d79#diff-fa8bd12854575a3f2897258a6d9eb254442d9696ff0ae8dc82b9e04ae4dd527d

I believe that patch is a compile work-around rather than a fix, right? All it does is make our kernel 6.12.7 builds behave like kernel < 6.8.0 and use stub declarations so the compilation succeeds.

@@ -8,6 +8,7 @@
#define _ACPI_AMD_WBRF_H

#if LINUX_VERSION_IS_GEQ(6,8,0)
#undef _ACPI_AMD_WBRF_H
Copy link

@namiltd namiltd Jan 2, 2025

Choose a reason for hiding this comment

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

Maybe it would be easier to just move line 8 (#define _ACPI_AMD_WBRF_H ) to line 14 (before #include <linux/device.h>)?

-#define _ACPI_AMD_WBRF_H

 #if LINUX_VERSION_IS_GEQ(6,8,0)
 #include_next <linux/acpi_amd_wbrf.h>
 #else
+#define _ACPI_AMD_WBRF_H
 #include <linux/device.h>

Copy link
Author

Choose a reason for hiding this comment

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

Actually, your suggestion is exactly what I used to test the fix when first trouble-shooting the problem, but while minimal it suffers from being a little confusing and opaque. However, I believe the PR code is clearer and makes both the problem and solution obvious (i.e. the same guard in the #include_next header causes it to be skipped).

Copy link

@namiltd namiltd Jan 3, 2025

Choose a reason for hiding this comment

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

In my opinion, less clear is
#define _ACPI_AMD_WBRF_H
and then after it
#undef _ACPI_AMD_WBRF_H

Copy link
Author

Choose a reason for hiding this comment

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

Having the standard guard

#ifndef _ACPI_AMD_WBRF_H
#define _ACPI_AMD_WBRF_H

at the beginning reminds that it's also present in the #include_next version of acpi_amd_wbrf.h, and hence the reason for the following #undef is immediately clear.

I'd just like the problem fixed, so I'm happy to leave it to the maintainer @nbd168 to decide on matters of opinion. 😃

@guidosarducci
Copy link
Author

Ping @nbd168. Any thoughts on this? Or openwrt/openwrt#17456? Appreciate if you're able to take a look.

@namiltd
Copy link

namiltd commented Jan 14, 2025

Fixed in openwrt/openwrt@7bf3bc8

@guidosarducci guidosarducci deleted the master-fix-wbrf-include branch January 14, 2025 23:53
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