[PATCH v1] HID: i2c-hid: use print_hex_dump_debug to print report descriptor

[PATCH v1] HID: i2c-hid: use print_hex_dump_debug to print report descriptor

[Riwen Lu]

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);
 
 	ret = hid_parse_report(hid, rdesc, rsize);
 	if (!use_override)
-- 
2.25.1

Re: [PATCH v1] HID: i2c-hid: use print_hex_dump_debug to print report descriptor

[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);

But that would dump it unconditionally, while i2c_hid_dbg() is 
conditional.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v1] HID: i2c-hid: use print_hex_dump_debug to print report descriptor

[Riwen Lu]

在 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);
> 
> 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().

Re: [PATCH v1] HID: i2c-hid: use print_hex_dump_debug to print report descriptor

[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.

>>
>> 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

Re: [PATCH v1] HID: i2c-hid: use print_hex_dump_debug to print report descriptor

[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
> 

[PATCH v2] HID: i2c-hid: use print_hex_dump_debug to print report descriptor

[Riwen Lu]

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>

---
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");
+	print_hex_dump_debug("Report Descriptor: ", DUMP_PREFIX_OFFSET, 16, 1,
+			rdesc, rsize, false);
 
 	ret = hid_parse_report(hid, rdesc, rsize);
 	if (!use_override)
-- 
2.25.1

Re: [PATCH v2] HID: i2c-hid: use print_hex_dump_debug to print report descriptor

[Rahul Rameshbabu]

On Wed, 16 Aug, 2023 16:38:19 +0800 "Riwen Lu" <luriwen@hotmail.com> 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>
>
> ---
> 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.

  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

Re: [PATCH v2] HID: i2c-hid: use print_hex_dump_debug to print report descriptor

[Riwen Lu]

在 2023/8/17 12:25, Rahul Rameshbabu 写道:
> 
> On Wed, 16 Aug, 2023 16:38:19 +0800 "Riwen Lu" <luriwen@hotmail.com> 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>
>>
>> ---
>> 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.
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
> 

Re: [PATCH v2] HID: i2c-hid: use print_hex_dump_debug to print report descriptor

[Rahul Rameshbabu]

On Wed, 23 Aug, 2023 16:03:46 +0800 "Riwen Lu" <luriwen@hotmail.com> wrote:
> 在 2023/8/17 12:25, Rahul Rameshbabu 写道:
>>
>> On Wed, 16 Aug, 2023 16:38:19 +0800 "Riwen Lu" <luriwen@hotmail.com> wrote:
>>> From: Riwen Lu <luriwen@kylinos.cn>
>>>
>>> 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@kylinos.cn>
>>>
>>> ---
>>> 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

[PATCH v3] HID: i2c-hid: use print_hex_dump_debug to print report descriptor

[Riwen Lu]

From: Riwen Lu <luriwen@kylinos.cn>

The format '%*ph' prints up to 64 bytes long as a hex string with ' '
separator. 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>

---
v1->v2:
 - Add a prefix for the hex dump.

v2->v3:
 - Print the size of report descriptor.
---
 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..dd69abdd1f0d 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 size: %#x\n", rsize);
+	print_hex_dump_debug("Report Descriptor: ", DUMP_PREFIX_OFFSET, 16, 1,
+			rdesc, rsize, false);
 
 	ret = hid_parse_report(hid, rdesc, rsize);
 	if (!use_override)
-- 
2.25.1

Re: [PATCH v3] HID: i2c-hid: use print_hex_dump_debug to print report descriptor

[Rahul Rameshbabu]

On Fri, 01 Sep, 2023 16:33:56 +0800 "Riwen Lu" <luriwen@hotmail.com> wrote:
> From: Riwen Lu <luriwen@kylinos.cn>
>
> The format '%*ph' prints up to 64 bytes long as a hex string with ' '
> separator. 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>
>
> ---
> v1->v2:
>  - Add a prefix for the hex dump.
>
> v2->v3:
>  - Print the size of report descriptor.
> ---
>  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..dd69abdd1f0d 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 size: %#x\n", rsize);
> +	print_hex_dump_debug("Report Descriptor: ", DUMP_PREFIX_OFFSET, 16, 1,
> +			rdesc, rsize, false);
>
>  	ret = hid_parse_report(hid, rdesc, rsize);
>  	if (!use_override)

Thanks for the new version.

Reviewed-by: Rahul Rameshbabu <sergeantsagara@protonmail.com>