[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/3] hw/block/pflash: Remove dynamic field width from trac
Re: [PATCH v2 1/3] hw/block/pflash: Remove dynamic field width from trace events
Mon, 18 Nov 2019 13:21:27 -0600
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.1
On 11/14/19 3:26 PM, Philippe Mathieu-Daudé wrote:
- trace_pflash_data_read(offset, width << 1, ret);
+ trace_pflash_data_read(offset, width << 3, ret);
Umm, why is width changing? That's not mentioned in the commit message.
Previously it was used to set the format width: [1, 2, 4] -> [2, 4, 8].
We usually log the width in byte (accessed at memory location) or bits
(used by the bus). When using this device I'm custom to think in bus
Regardless whichever format we prefer, a change is needed.
Do you prefer using a "-bit" suffix? As
"offset:0x%04"PRIx64" width:%d-bit value:0x%04x cmd:0x%02x wcycle:%u"
I can also simply remove this information. Ideally I'd revert this patch
once the we get this format parsable by the SystemTap backend.
Reporting either 'width:8-bit'/'width:16-bit' (explicit bits) or
'width:1'/'width:2' (implying byte) is fine by me. Showing a bus width
in bytes adequately explains why you are using <<3 (aka converting bits
to bytes), and how it compares to the previous <<1 (converting bits to
number of hex characters). But whichever you pick (tracing bit width
vs. byte width, and how it differs from previous usage of width as
output-character count), documenting it in the commit message will make
life easier to understand the change.
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[PATCH v2 2/3] hw/mips/gt64xxx: Remove dynamic field width from trace events, Philippe Mathieu-Daudé, 2019/11/08
[PATCH v2 3/3] trace: Forbid dynamic field width in event format, Philippe Mathieu-Daudé, 2019/11/08