On Wed, 23 Aug, 2023 16:03:46 +0800 "Riwen Lu" <luriwen@xxxxxxxxxxx> wrote: > 在 2023/8/17 12:25, Rahul Rameshbabu 写道: >> >> On Wed, 16 Aug, 2023 16:38:19 +0800 "Riwen Lu" <luriwen@xxxxxxxxxxx> wrote: >>> From: Riwen Lu <luriwen@xxxxxxxxxx> >>> >>> The format '%*ph' print up to 64 bytes long as a hex string with ' ' Cosmetic but if you are sending a v3 anyways, s/print/prints. >>> sepatator. Usually the size of report descriptor is larger than 64 s/sepatator/separator >>> bytes, so consider using print_hex_dump_debug to print out all of it for >>> better debugging. >>> >>> Signed-off-by: Riwen Lu <luriwen@xxxxxxxxxx> >>> >>> --- >>> v1->v2: >>> - Add a prefix for the hex dump. >>> --- >>> 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..fd82e9042da5 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"); >> >> Instead of just indicating that the report descriptor dump begins with >> the above print, I think it makes more sense for the print to be changed >> to a pair of begin/end or "cut here" prints similar to what you see in >> oops messages. This will help individuals reading reports copied by bug >> reporters validate that the complete descriptor dump is present. >> >> Something along the lines of what is done in sound/soc/sof/debug.c. >> > I don't think it's necessary to add a pair of begin/end prints like > that. However, I can print out the size of the report descriptor through > i2c_hid_dbg. And print_hex_dump_debug prints each line with a "Report > Descriptor" prefix and an offset, so it's easy to know if the descriptor > dump is complete. > The dump message is similar to the following. Yeah, I agree with this. My main concern was the fact that the i2c_hid_dbg was not adding any value as-is aside from indicating what driver is causing the dump (which could be appended to print_hex_dump_debug). I like adding the size in the print. > i2c_hid i2c-PNP0C50:00: Report Descriptor size: 0x144 > Report Descriptor: 00000000: 05 01 09 02 a1 01 85 01 09 01 a1 00 05 09 19 01 > Report Descriptor: 00000010: 29 02 15 00 25 01 75 01 95 02 81 02 95 06 81 03 > Report Descriptor: 00000020: 05 01 09 30 09 31 09 38 15 81 25 7f 75 08 95 03 > Report Descriptor: 00000030: 81 06 c0 c0 05 0d 09 05 a1 01 85 04 09 22 a1 02 > Report Descriptor: 00000040: 15 00 25 01 09 47 09 42 95 02 75 01 81 02 95 01 > Report Descriptor: 00000050: 75 03 25 02 09 51 81 02 75 01 95 03 81 03 05 01 > Report Descriptor: 00000060: 15 00 26 5b 05 75 10 55 0d 65 11 09 30 35 00 46 > Report Descriptor: 00000070: 46 2a 95 01 81 02 46 59 17 26 f4 02 09 31 81 02 > Report Descriptor: 00000080: 05 0d 15 00 25 64 95 03 c0 55 0c 66 01 10 47 ff > Report Descriptor: 00000090: ff 00 00 27 ff ff 00 00 75 10 95 01 09 56 81 02 > Report Descriptor: 000000a0: 09 54 25 7f 95 01 75 08 81 02 05 09 09 01 25 01 > Report Descriptor: 000000b0: 75 01 95 01 81 02 95 07 81 03 09 c5 75 20 95 01 > Report Descriptor: 000000c0: 81 03 05 0d 85 02 09 55 09 59 75 04 95 02 25 0f > Report Descriptor: 000000d0: b1 02 85 07 09 60 75 01 95 01 15 00 25 01 b1 02 > Report Descriptor: 000000e0: 95 07 b1 03 06 00 ff 85 08 09 c5 15 00 26 ff 00 > Report Descriptor: 000000f0: 75 08 96 00 01 b1 02 c0 05 0d 09 0e a1 01 85 03 > Report Descriptor: 00000100: 09 22 a1 02 09 52 15 00 25 0a 75 08 95 01 b1 02 > Report Descriptor: 00000110: c0 09 22 a1 00 85 05 09 57 09 58 75 01 95 02 25 > Report Descriptor: 00000120: 03 b1 02 95 06 b1 03 c0 c0 06 00 ff 09 01 a1 01 > Report Descriptor: 00000130: 15 00 26 ff 00 75 08 85 06 95 3f 09 01 81 02 09 > Report Descriptor: 00000140: 01 91 02 c0 > > Thanks. > >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/sound/soc/sof/debug.c?id=4853c74bd7ab7fdb83f319bd9ace8a08c031e9b6#n407 >> >>> + print_hex_dump_debug("Report Descriptor: ", DUMP_PREFIX_OFFSET, 16, 1, >>> + rdesc, rsize, false); >>> >> -- Thanks, Rahul Rameshbabu
