Hello,
On 20 February 2018 at 10:27, Stefan Schmidt <stefan@xxxxxxxxxxxxxxx> wrote:
> Hello.
>
>
> On 02/20/2018 12:59 AM, Xue Liu wrote:
>> Hello
>>
>> On 19 February 2018 at 23:37, Stefan Schmidt <stefan@xxxxxxxxxxxxxxx> wrote:
>>> Hello.
>>>
>>>
>>> On 02/19/2018 10:40 PM, Xue Liu wrote:
>>>> Hello Stefan,
>>>>
>>>> Thanks for the second review.
>>>>
>>>> On 19 February 2018 at 17:31, Stefan Schmidt <stefan@xxxxxxxxxxxxxxx> wrote:
>>>>> Hello.
>>>>>
>>>>> On Mon, 2018-02-19 at 11:51, Xue Liu wrote:
>>>>>
>>>>>> +
>>>>>> +#ifdef DEBUG
>>>>>> + print_hex_dump(KERN_INFO, "mcr20a rx: ", DUMP_PREFIX_OFFSET, 16, 1,
>>>>>> + lp->rx_buf, len, 0);
>>>>>> + pr_info("mcr20a rx: lqi: %02hhx\n", lp->rx_lqi[0]);
>>>>>> +#endif
>>>>> Here, as well as in the corresponding TX hex_dump call I wonder how to better
>>>>> make use of it. Recompiling the driver to get the dump is not really nice.
>>>>> Having a way to have this enabled during runtime might be better. And across
>>>>> all drivers. Just thinking out loud here. Not saying you need to be he one
>>>>> implementing it. What do you think?
>>>>>
>>>>>
>>>> using module parameters may a solution.
>>> This would be an option but I was thinking more about towards a tracepoints like approach where we can dynamically enable or disable the
>>> hexdump or other debug functionality. Loading and unloading the module to change this is a bit cumbersome. This code might not even be a
>>> module at all but compiled in.
>>>
>>> I need to check what the kernel infra offers here and how other subsystems use it.
>>>
>> Do you mean "dynamic debug"
>> https://www.kernel.org/doc/html/v4.11/admin-guide/dynamic-debug-howto.html
>> ?
>> I saw there is all a version for print_hex_dump.
>
> Indeed, that would be a good way of doing it. Switch to print_hex_dump_debug() and pr_debug and eliminate the ifdef.
> That way it is always compiled in but off by default and can be dynamically enabled during runtime.
>
> Are you ok with changing this or would you prefer to stay with your current construct?
>
I will change the functions to print_hex_dump_debug() and pr_debug()
and post a new patch.
> regards
> Stefan Schmidt
Regards,
Xue Liu
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
