Messages in this thread | | | Date | Tue, 15 Aug 2023 14:58:41 +0800 | Subject | Re: [PATCH v1] HID: i2c-hid: use print_hex_dump_debug to print report descriptor | From | Riwen Lu <> |
| |
在 2023/8/15 14:35, Rahul Rameshbabu 写道: > > On Tue, 15 Aug, 2023 14:02:40 +0800 "Riwen Lu" <luriwen@hotmail.com> wrote: >> 在 2023/8/14 17:26, Jiri Kosina 写道: >>> On Mon, 3 Jul 2023, Riwen Lu wrote: >>> >>>> From: Riwen Lu <luriwen@kylinos.cn> >>>> >>>> The format '%*ph' print up to 64 bytes long as a hex string with ' ' >>>> sepatator. Usually the size of report descriptor is larger than 64 >>>> bytes, so consider using print_hex_dump_debug to print out all of it for >>>> better debugging. >>>> >>>> Signed-off-by: Riwen Lu <luriwen@kylinos.cn> >>>> --- >>>> drivers/hid/i2c-hid/i2c-hid-core.c | 4 +++- >>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c >>>> index efbba0465eef..8e97fc01c852 100644 >>>> --- a/drivers/hid/i2c-hid/i2c-hid-core.c >>>> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c >>>> @@ -772,7 +772,9 @@ static int i2c_hid_parse(struct hid_device *hid) >>>> } >>>> } >>>> >>>> - i2c_hid_dbg(ihid, "Report Descriptor: %*ph\n", rsize, rdesc); >>>> + i2c_hid_dbg(ihid, "Report Descriptor\n"); >>>> + print_hex_dump_debug(" ", DUMP_PREFIX_OFFSET, 16, 1, >>>> + rdesc, rsize, false); > > Maybe it makes sense to use a prefix for the hex dump that is easy to > trace rather than padding whitespace? This looks good when you do not > see any other kernel message log lines get interlaced when written. > However, if you have a lot of concurrent kernel message output, I think > it can be tough to piece together the lines of the dump with this > prefix. Just my opinion. > That's a good opinion. I'll make a v2 version.
Thanks. >>> >>> But that would dump it unconditionally, while i2c_hid_dbg() is >>> conditional. >>> >> Function print_hex_dump_debug() dump messages is as conditional as >> i2c_hid_dbg(). >> >> The function i2c_hid_dbg() defines as follows: >> #define i2c_hid_dbg(ihid, ...) dev_dbg(&(ihid)->client->dev, __VA_ARGS__) >> >> dev_dbg() depends on the same macro as print_hex_dump_debug(). > > I agree with this point. Both dev_dbg and print_hex_dump_debug are noops > if neither CONFIG_DYNAMIC_DEBUG is set or DEBUG is defined. > > -- > Thanks, > > Rahul Rameshbabu >
| |