Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38006 )
Change subject: usbdebug: Fix printk conversion ......................................................................
usbdebug: Fix printk conversion
Change-Id: I0dba96004de264fe1faf5485fb677a6b05123bba Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/drivers/usb/ehci_debug.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/38006/1
diff --git a/src/drivers/usb/ehci_debug.c b/src/drivers/usb/ehci_debug.c index 318dfe7..b31b563 100644 --- a/src/drivers/usb/ehci_debug.c +++ b/src/drivers/usb/ehci_debug.c @@ -452,7 +452,7 @@ info->ehci_debug = ehci_bar + offset; info->ep_pipe[0].status |= DBGP_EP_NOT_PRESENT;
- dprintk(BIOS_INFO, "ehci_bar: 0x%x debug_offset 0x%x\n", ehci_bar, offset); + dprintk(BIOS_INFO, "ehci_bar: 0x%zx debug_offset 0x%x\n", ehci_bar, offset);
ehci_caps = (struct ehci_caps *)ehci_bar; ehci_regs = (struct ehci_regs *)(ehci_bar +
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38006 )
Change subject: usbdebug: Fix printk conversion ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38006/1/src/drivers/usb/ehci_debug.... File src/drivers/usb/ehci_debug.c:
https://review.coreboot.org/c/coreboot/+/38006/1/src/drivers/usb/ehci_debug.... PS1, Line 455: dprintk(BIOS_INFO, "ehci_bar: 0x%zx debug_offset 0x%x\n", ehci_bar, offset); Uint pointers are not directly related with size_t. Would prefer `lx` instead (we noticed that for all supported platforms `long` matches pointers in size when writing generic stdint.h/inttypes.h) or just use PRIxPTR which can't be bikeshedded.
Hello Arthur Heymans, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38006
to look at the new patch set (#2).
Change subject: usbdebug: Fix printk conversion ......................................................................
usbdebug: Fix printk conversion
Change-Id: I0dba96004de264fe1faf5485fb677a6b05123bba Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/drivers/usb/ehci_debug.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/38006/2
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38006 )
Change subject: usbdebug: Fix printk conversion ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38006/1/src/drivers/usb/ehci_debug.... File src/drivers/usb/ehci_debug.c:
https://review.coreboot.org/c/coreboot/+/38006/1/src/drivers/usb/ehci_debug.... PS1, Line 455: dprintk(BIOS_INFO, "ehci_bar: 0x%zx debug_offset 0x%x\n", ehci_bar, offset);
Uint pointers are not directly related with size_t. Would prefer `lx` instead […]
Done
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38006 )
Change subject: usbdebug: Fix printk conversion ......................................................................
Patch Set 2: Code-Review+1
Did the compiler warn about that? Does this also work with 64-bit compilation?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38006 )
Change subject: usbdebug: Fix printk conversion ......................................................................
Patch Set 2:
Did the compiler warn about that?
I don't think we have format warnings enabled. If we have, a compiler should have warned in the 64-bit case. To be honest, I have no idea how varargs are implemented for 64-bit targets. If it's not aligning every argument to at least 64 bits, it would have printed the wrong numbers.
Does this also work with 64-bit compilation?
`%lx` is safe for all our targets, 32 and 64 bits, because a long matches the pointer size for all of them.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38006 )
Change subject: usbdebug: Fix printk conversion ......................................................................
Patch Set 2: Code-Review+2
Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38006 )
Change subject: usbdebug: Fix printk conversion ......................................................................
usbdebug: Fix printk conversion
Change-Id: I0dba96004de264fe1faf5485fb677a6b05123bba Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/38006 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Nico Huber nico.h@gmx.de --- M src/drivers/usb/ehci_debug.c 1 file changed, 1 insertion(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Paul Menzel: Looks good to me, but someone else must approve
diff --git a/src/drivers/usb/ehci_debug.c b/src/drivers/usb/ehci_debug.c index 318dfe7..5a3f2a6 100644 --- a/src/drivers/usb/ehci_debug.c +++ b/src/drivers/usb/ehci_debug.c @@ -452,7 +452,7 @@ info->ehci_debug = ehci_bar + offset; info->ep_pipe[0].status |= DBGP_EP_NOT_PRESENT;
- dprintk(BIOS_INFO, "ehci_bar: 0x%x debug_offset 0x%x\n", ehci_bar, offset); + dprintk(BIOS_INFO, "ehci_bar: 0x%lx debug_offset 0x%x\n", ehci_bar, offset);
ehci_caps = (struct ehci_caps *)ehci_bar; ehci_regs = (struct ehci_regs *)(ehci_bar +
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38006 )
Change subject: usbdebug: Fix printk conversion ......................................................................
Patch Set 3:
Patch Set 2: Code-Review+1
Did the compiler warn about that? Does this also work with 64-bit compilation?
src/drivers/usb/ehci_debug.c:455:21: error: format '%x' expects argument of type 'unsigned int', but argument 3 has type 'uintptr_t' {aka 'long unsigned int'} [-Werror=format=]
-
Kyösti Mälkki (Code Review) -
Nico Huber (Code Review)
-
Paul Menzel (Code Review)