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 compilation warning with 64 bit system #413

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Ansuel
Copy link

@Ansuel Ansuel commented May 9, 2023

Use %zu and %zd where possible for ssize_t and size_t.
Use PTR_ERR to correctly convert to negative error.
Use universal pointer to support both 32 and 64bit systems.

Fix compilation warning:
/home/ansuel/openwrt-ansuel/openwrt/build_dir/target-aarch64_cortex-a53_musl/linux-ipq807x_generic/mwlwifi-2023-04-29-6a436714/hif/fwcmd.c: In function 'mwl_fwcmd_get_fw_core_dump': /home/ansuel/openwrt-ansuel/openwrt/build_dir/target-aarch64_cortex-a53_musl/linux-ipq807x_generic/mwlwifi-2023-04-29-6a436714/hif/fwcmd.c:3608:31: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
3608 | (const void *)((u32)pcmd +
| ^
In file included from ./include/linux/device.h:15,
from /home/ansuel/openwrt-ansuel/openwrt/staging_dir/target-aarch64_cortex-a53_musl/usr/include/mac80211-backport/linux/device.h:3,
from ./include/linux/dma-mapping.h:7,
from /home/ansuel/openwrt-ansuel/openwrt/staging_dir/target-aarch64_cortex-a53_musl/usr/include/mac80211-backport/linux/dma-mapping.h:3,
from ./include/linux/skbuff.h:31,
from /home/ansuel/openwrt-ansuel/openwrt/staging_dir/target-aarch64_cortex-a53_musl/usr/include/mac80211-backport/linux/skbuff.h:3,
from ./include/linux/if_ether.h:19,
from /home/ansuel/openwrt-ansuel/openwrt/staging_dir/target-aarch64_cortex-a53_musl/usr/include/mac80211-backport/linux/if_ether.h:3,
from ./include/linux/etherdevice.h:20,
from /home/ansuel/openwrt-ansuel/openwrt/staging_dir/target-aarch64_cortex-a53_musl/usr/include/mac80211-backport/linux/etherdevice.h:3,
from /home/ansuel/openwrt-ansuel/openwrt/build_dir/target-aarch64_cortex-a53_musl/linux-ipq807x_generic/mwlwifi-2023-04-29-6a436714/hif/pcie/tx_ndp.c:20:
/home/ansuel/openwrt-ansuel/openwrt/build_dir/target-aarch64_cortex-a53_musl/linux-ipq807x_generic/mwlwifi-2023-04-29-6a436714/hif/pcie/tx_ndp.c: In function 'pcie_tx_init_ndp':
/home/ansuel/openwrt-ansuel/openwrt/build_dir/target-aarch64_cortex-a53_musl/linux-ipq807x_generic/mwlwifi-2023-04-29-6a436714/hif/pcie/tx_ndp.c:338:38: error: format '%d' expects argument of type 'int', but argument 3 has type 'long unsigned int' [-Werror=format=]
338 | wiphy_err(hw->wiphy, "driver data is not enough: %d (%d)\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##VA_ARGS);
| ^~~
./include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt'
144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##VA_ARGS)
| ^~~~~~~
/home/ansuel/openwrt-ansuel/openwrt/staging_dir/target-aarch64_cortex-a53_musl/usr/include/mac80211/net/cfg80211.h:8828:9: note: in expansion of macro 'dev_err'
8828 | dev_err(&(wiphy)->dev, format, ##args)
| ^~~~~~~
/home/ansuel/openwrt-ansuel/openwrt/build_dir/target-aarch64_cortex-a53_musl/linux-ipq807x_generic/mwlwifi-2023-04-29-6a436714/hif/pcie/tx_ndp.c:338:17: note: in expansion of macro 'wiphy_err'
338 | wiphy_err(hw->wiphy, "driver data is not enough: %d (%d)\n",
| ^~~~~~~~~
/home/ansuel/openwrt-ansuel/openwrt/build_dir/target-aarch64_cortex-a53_musl/linux-ipq807x_generic/mwlwifi-2023-04-29-6a436714/hif/pcie/tx_ndp.c:338:67: note: format string is defined here
338 | wiphy_err(hw->wiphy, "driver data is not enough: %d (%d)\n",
| ~^
| |
| int
| %ld
/home/ansuel/openwrt-ansuel/openwrt/build_dir/target-aarch64_cortex-a53_musl/linux-ipq807x_generic/mwlwifi-2023-04-29-6a436714/hif/pcie/tx_ndp.c:338:38: error: format '%d' expects argument of type 'int', but argument 4 has type 'long unsigned int' [-Werror=format=]
338 | wiphy_err(hw->wiphy, "driver data is not enough: %d (%d)\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##VA_ARGS);
| ^~~
./include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt'
144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##VA_ARGS)
| ^~~~~~~
/home/ansuel/openwrt-ansuel/openwrt/staging_dir/target-aarch64_cortex-a53_musl/usr/include/mac80211/net/cfg80211.h:8828:9: note: in expansion of macro 'dev_err'
8828 | dev_err(&(wiphy)->dev, format, ##args)
| ^~~~~~~
/home/ansuel/openwrt-ansuel/openwrt/build_dir/target-aarch64_cortex-a53_musl/linux-ipq807x_generic/mwlwifi-2023-04-29-6a436714/hif/pcie/tx_ndp.c:338:17: note: in expansion of macro 'wiphy_err'
338 | wiphy_err(hw->wiphy, "driver data is not enough: %d (%d)\n",
| ^~~~~~~~~
/home/ansuel/openwrt-ansuel/openwrt/build_dir/target-aarch64_cortex-a53_musl/linux-ipq807x_generic/mwlwifi-2023-04-29-6a436714/hif/pcie/tx_ndp.c:338:71: note: format string is defined here
338 | wiphy_err(hw->wiphy, "driver data is not enough: %d (%d)\n",
| ~^
| |
| int
| %ld
CC [M] /home/ansuel/openwrt-ansuel/openwrt/build_dir/target-aarch64_cortex-a53_musl/linux-ipq807x_generic/mwlwifi-2023-04-29-6a436714/debugfs.o
In file included from ./include/linux/device.h:15,
from /home/ansuel/openwrt-ansuel/openwrt/staging_dir/target-aarch64_cortex-a53_musl/usr/include/mac80211-backport/linux/device.h:3,
from ./include/linux/dma-mapping.h:7,
from /home/ansuel/openwrt-ansuel/openwrt/staging_dir/target-aarch64_cortex-a53_musl/usr/include/mac80211-backport/linux/dma-mapping.h:3,
from ./include/linux/skbuff.h:31,
from /home/ansuel/openwrt-ansuel/openwrt/staging_dir/target-aarch64_cortex-a53_musl/usr/include/mac80211-backport/linux/skbuff.h:3,
from ./include/linux/if_ether.h:19,
from /home/ansuel/openwrt-ansuel/openwrt/staging_dir/target-aarch64_cortex-a53_musl/usr/include/mac80211-backport/linux/if_ether.h:3,
from ./include/linux/etherdevice.h:20,
from /home/ansuel/openwrt-ansuel/openwrt/staging_dir/target-aarch64_cortex-a53_musl/usr/include/mac80211-backport/linux/etherdevice.h:3,
from /home/ansuel/openwrt-ansuel/openwrt/build_dir/target-aarch64_cortex-a53_musl/linux-ipq807x_generic/mwlwifi-2023-04-29-6a436714/hif/pcie/pcie.c:19:
/home/ansuel/openwrt-ansuel/openwrt/build_dir/target-aarch64_cortex-a53_musl/linux-ipq807x_generic/mwlwifi-2023-04-29-6a436714/hif/pcie/pcie.c: In function 'pcie_bf_mimo_ctrl_decode':
/home/ansuel/openwrt-ansuel/openwrt/build_dir/target-aarch64_cortex-a53_musl/linux-ipq807x_generic/mwlwifi-2023-04-29-6a436714/hif/pcie/pcie.c:1325:37: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
1325 | filename, (unsigned int)fp_data);
| ^
./include/linux/dev_printk.h:110:37: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##VA_ARGS);
| ^~~~~~~~~~~
/home/ansuel/openwrt-ansuel/openwrt/staging_dir/target-aarch64_cortex-a53_musl/usr/include/mac80211/net/cfg80211.h:8828:9: note: in expansion of macro 'dev_err'
8828 | dev_err(&(wiphy)->dev, format, ##args)
| ^~~~~~~
/home/ansuel/openwrt-ansuel/openwrt/build_dir/target-aarch64_cortex-a53_musl/linux-ipq807x_generic/mwlwifi-2023-04-29-6a436714/hif/pcie/pcie.c:1324:17: note: in expansion of macro 'wiphy_err'
1324 | wiphy_err(priv->hw->wiphy, "Error opening %s! %x\n",
| ^~~~~~~~~
cc1: all warnings being treated as errors
make[4]: *** [scripts/Makefile.build:289: /home/ansuel/openwrt-ansuel/openwrt/build_dir/target-aarch64_cortex-a53_musl/linux-ipq807x_generic/mwlwifi-2023-04-29-6a436714/hif/pcie/tx_ndp.o] Error 1
make[4]: *** Waiting for unfinished jobs....
cc1: all warnings being treated as errors
make[4]: *** [scripts/Makefile.build:289: /home/ansuel/openwrt-ansuel/openwrt/build_dir/target-aarch64_cortex-a53_musl/linux-ipq807x_generic/mwlwifi-2023-04-29-6a436714/hif/pcie/pcie.o] Error 1
/home/ansuel/openwrt-ansuel/openwrt/build_dir/target-aarch64_cortex-a53_musl/linux-ipq807x_generic/mwlwifi-2023-04-29-6a436714/debugfs.c: In function 'mwl_debugfs_regrdwr_read':
/home/ansuel/openwrt-ansuel/openwrt/build_dir/target-aarch64_cortex-a53_musl/linux-ipq807x_generic/mwlwifi-2023-04-29-6a436714/debugfs.c:1335:43: error: format '%d' expects argument of type 'int', but argument 4 has type 'ssize_t' {aka 'long int'} [-Werror=format=]
1335 | "error: %d(%u 0x%08x 0x%08x)\n",
| ~^
| |
| int
| %ld
1336 | ret, priv->reg_type, priv->reg_offset,
| ~~~
| |
| ssize_t {aka long int}
cc1: all warnings being treated as errors


@kaloz simple pr. We are enforcing WERROR on OpenWRT and these came up with building on mvebu cortexa72

debugfs.c Outdated Show resolved Hide resolved
hif/fwcmd.c Outdated
@@ -3604,7 +3604,7 @@ int mwl_fwcmd_get_fw_core_dump(struct ieee80211_hw *hw,
core_dump->size_kb = pcmd->cmd_data.coredump.size_kb;
core_dump->flags = pcmd->cmd_data.coredump.flags;
memcpy(buff,
(const void *)((u32)pcmd +
(const void *)((uintptr_t)pcmd +
Copy link

Choose a reason for hiding this comment

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

pcmd is already a pointer. The cast to u32 in the original code effectively truncates the higher 32-bit..
You can remove the cast.

Copy link
Author

Choose a reason for hiding this comment

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

@LGA1150 i may be totally wrong but without casting think we would add at offset of struct hostcmd_cmd_get_fw_core_dump and not single address of 32 or 64 bit.

Copy link

@LGA1150 LGA1150 May 12, 2023

Choose a reason for hiding this comment

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

My bad, I didn't notice the sizeof addition. But the offset calculation looks ugly anyway.

struct hostcmd_cmd_get_fw_core_dump {
	struct hostcmd_header cmd_hdr;
	union {
		struct coredump_cmd coredump;
		struct debug_mem_cmd debug_mem;
	} cmd_data;
	/*Buffer where F/W Copies the Core Dump*/
	char buffer[MAX_CORE_DUMP_BUFFER];
} __packed;

struct hostcmd_cmd_get_fw_core_dump_ {
	struct hostcmd_header cmd_hdr;
	union {
		struct coredump_cmd coredump;
		struct debug_mem_cmd debug_mem;
	} cmd_data;
} __packed;

So, sizeof(struct hostcmd_cmd_get_fw_core_dump) - sizeof(struct hostcmd_cmd_get_fw_core_dump_) is the offset of buffer in struct hostcmd_cmd_get_fw_core_dump. One can use offsetof to get the offset. And in this case, pcmd is already struct hostcmd_cmd_get_fw_core_dump*, so pcmd->buffer is enough.

Copy link
Author

Choose a reason for hiding this comment

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

yep was aware of the offsetof macro but I didn't want to modify this too much. Will check if we can directly use buffer

hif/pcie/pcie.c Outdated Show resolved Hide resolved
hif/pcie/tx_ndp.c Outdated Show resolved Hide resolved
@Ansuel Ansuel force-pushed the fix-64bit branch 2 times, most recently from afffd2b to ed4422e Compare May 11, 2023 14:04
@neheb
Copy link
Contributor

neheb commented Nov 29, 2023

This branch has conflicts that must be resolved

Ansuel added 5 commits May 12, 2024 14:44
Simplify coredump memcpy in mwl_fwcmd_get_fw_core_dump. Instead of doing
fragile address additions, just access the buffer member in pcmd and
reference the pointer.

This fix a compilation warning in 64Bit system:

/home/ansuel/openwrt-ansuel/openwrt/build_dir/target-aarch64_cortex-a53_musl/linux-ipq807x_generic/mwlwifi-2023-04-29-6a436714/hif/fwcmd.c: In function 'mwl_fwcmd_get_fw_core_dump': /home/ansuel/openwrt-ansuel/openwrt/build_dir/target-aarch64_cortex-a53_musl/linux-ipq807x_generic/mwlwifi-2023-04-29-6a436714/hif/fwcmd.c:3608:31: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
3608 | (const void *)((u32)pcmd +
| ^

Signed-off-by: Christian Marangi <[email protected]>
Correctly use PTR_ERR instead of cast to unsigned int in
pcie_bf_mimo_ctrl_decode if fp_data pointer contains errors.

This fix a compilation warning on 64Bit:

/home/ansuel/openwrt-ansuel/openwrt/build_dir/target-aarch64_cortex-a53_musl/linux-ipq807x_generic/mwlwifi-2023-04-29-6a436714/hif/pcie/pcie.c:1325:37: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
1325 | filename, (unsigned int)fp_data);
| ^

Signed-off-by: Christian Marangi <[email protected]>
Use %zu and %zu for ssize_t and size_t to fix compilation warning on
64Bit.

Signed-off-by: Christian Marangi <[email protected]>
Fix debugfs compilation warning on 64Bit mwl_debugfs_info_read by
casing for uintptr_t and use %zx.

Signed-off-by: Christian Marangi <[email protected]>
…_ndp

Use BUILD_BUG_ON instead of checking size at runtime for
pcie_tx_init_ndp.

Signed-off-by: Christian Marangi <[email protected]>
@Ansuel
Copy link
Author

Ansuel commented May 12, 2024

@kaloz @LGA1150 can you guys recheck this? Rebased and updated with new warnings.

sizeof(tx_info->driver_data));
return -ENOMEM;
}
BUILD_BUG_ON(sizeof(struct pcie_tx_ctrl_ndp) >
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to use BUILD_BUG_ON_MSG instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, thank you for maintaining this message, it's an initialization function, it doesn't cost anything in terms of performance.

@@ -364,10 +364,10 @@ static ssize_t mwl_debugfs_info_read(struct file *file, char __user *ubuf,
"-----------------------=> address| address|qlen|fw_desc_cnt\n");
spin_lock_irqsave(&pcie_priv->tx_desc_lock, flags);
len += scnprintf(p + len, size - len,
"wcb_base0 : %x => %8x|%8p|%4d|%d\n", get_hw_spec->wcb_base0, *((unsigned int *)le32_to_cpu(get_hw_spec->wcb_base0)),(void *)*((unsigned int *)le32_to_cpu(get_hw_spec->wcb_base0)),skb_queue_len(&pcie_priv->txq[0]),pcie_priv->fw_desc_cnt[0]);
Copy link
Contributor

Choose a reason for hiding this comment

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

zx requires size_t.

for(i = 0; i < SYSADPT_TOTAL_TX_QUEUES - 1; i++)
len += scnprintf(p + len, size - len,
"wcb_base[%2d]: %x => %8x|%8p|%4d|%d\n", i, get_hw_spec->wcb_base[i], *((unsigned int *)le32_to_cpu(get_hw_spec->wcb_base[i])),(void *)*((unsigned int *)le32_to_cpu(get_hw_spec->wcb_base[i])),skb_queue_len(&pcie_priv->txq[i + 1]),pcie_priv->fw_desc_cnt[i + 1]);
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can delete this part :

*((unsigned int *)le32_to_cpu(get_hw_spec->wcb_base[i])),(void *)*((unsigned int *)le32_to_cpu(get_hw_spec->wcb_base[i])),

wcb_base must be an internal memory on the 88X8xxx chip, searching RAM is useless.

@@ -3622,11 +3622,7 @@ int mwl_fwcmd_get_fw_core_dump(struct ieee80211_hw *hw,
core_dump->context = pcmd->cmd_data.coredump.context;
Copy link
Contributor

Choose a reason for hiding this comment

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

You'd have to test this function, I'm not sure:

	pcmd->cmd_data.coredump.buffer = cpu_to_le32(priv->pphys_cmd_buf +
		sizeof(struct hostcmd_cmd_get_fw_core_dump) -
		sizeof(struct hostcmd_cmd_get_fw_core_dump_));

in
image
2 struct have exactly the same size.

struct coredump_cmd {
	__le32 context;
	__le32 buffer;
	__le32 buffer_len;
	__le16 size_kb;
	__le16 flags;
} __packed;

struct debug_mem_cmd {
	__le32 set;
	__le32 type;
	__le32 addr;
	__le32 val;
} __packed;

but

char buffer[MAX_CORE_DUMP_BUFFER];

Roughly speaking, for a union, if I calculate, it would be worth :

cpu_to_le32(priv->pphys_cmd_buf 
		+ sizeof(char buffer[MAX_CORE_DUMP_BUFFER]))

sounds strange
Thank you in advance,

hauke pushed a commit to neheb/openwrt that referenced this pull request Oct 19, 2024
Upstream patch updated to fix kernel 6.6 compilation. It was also split
up into 5. Do the same here.

The patches are taken from this upstreasm PR:
kaloz/mwlwifi#413

Renamed other patches so as to not overlap.

Signed-off-by: Rosen Penev <[email protected]>
Link: openwrt#15452
Signed-off-by: Hauke Mehrtens <[email protected]>
liudf0716 pushed a commit to liudf0716/openwrt that referenced this pull request Oct 22, 2024
Upstream patch updated to fix kernel 6.6 compilation. It was also split
up into 5. Do the same here.

The patches are taken from this upstreasm PR:
kaloz/mwlwifi#413

Renamed other patches so as to not overlap.

Signed-off-by: Rosen Penev <[email protected]>
Link: openwrt#15452
Signed-off-by: Hauke Mehrtens <[email protected]>
vincejv pushed a commit to vincejv/openwrt that referenced this pull request Oct 27, 2024
Upstream patch updated to fix kernel 6.6 compilation. It was also split
up into 5. Do the same here.

The patches are taken from this upstreasm PR:
kaloz/mwlwifi#413

Renamed other patches so as to not overlap.

Signed-off-by: Rosen Penev <[email protected]>
Link: openwrt#15452
Signed-off-by: Hauke Mehrtens <[email protected]>
vincejv pushed a commit to vincejv/openwrt that referenced this pull request Oct 27, 2024
Upstream patch updated to fix kernel 6.6 compilation. It was also split
up into 5. Do the same here.

The patches are taken from this upstreasm PR:
kaloz/mwlwifi#413

Renamed other patches so as to not overlap.

Signed-off-by: Rosen Penev <[email protected]>
Link: openwrt#15452
Signed-off-by: Hauke Mehrtens <[email protected]>
kimocoder pushed a commit to thisiskimocoder/openwrt that referenced this pull request Oct 28, 2024
Upstream patch updated to fix kernel 6.6 compilation. It was also split
up into 5. Do the same here.

The patches are taken from this upstreasm PR:
kaloz/mwlwifi#413

Renamed other patches so as to not overlap.

Signed-off-by: Rosen Penev <[email protected]>
Link: openwrt#15452
Signed-off-by: Hauke Mehrtens <[email protected]>
Vladdrako pushed a commit to Vladdrako/openwrt that referenced this pull request Dec 11, 2024
Upstream patch updated to fix kernel 6.6 compilation. It was also split
up into 5. Do the same here.

The patches are taken from this upstreasm PR:
kaloz/mwlwifi#413

Renamed other patches so as to not overlap.

Signed-off-by: Rosen Penev <[email protected]>
Link: openwrt#15452
Signed-off-by: Hauke Mehrtens <[email protected]>
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.

4 participants