All of lore.kernel.org
 help / color / mirror / Atom feed
From: Adrian Chadd <adrian@freebsd.org>
To: Kalle Valo <kvalo@qca.qualcomm.com>
Cc: Arend van Spriel <arend.vanspriel@broadcom.com>,
	"ath10k@lists.infradead.org" <ath10k@lists.infradead.org>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH] ath10k: add configurable debugging.
Date: Thu, 1 Jun 2017 06:33:38 -0700	[thread overview]
Message-ID: <CAJ-VmokfLW6RZyKZ4cKR7+cbTxwwa4aUZPm88Djv17Bo-oZ-0A@mail.gmail.com> (raw)
In-Reply-To: <87r2z3lu3j.fsf@kamboji.qca.qualcomm.com>

On 1 June 2017 at 06:24, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>
>> On 31-05-17 14:16, Kalle Valo wrote:
>>> Adrian Chadd <adrian@freebsd.org> writes:
>>>
>>>> This adds a few configurable debugging options:
>>>>
>>>> * driver debugging and tracing is now configurable per device
>>>> * driver debugging and tracing is now configurable at runtime
>>>> * the debugging / tracing is not run at all (besides a mask check)
>>>>   unless the specific debugging bitmap field is configured.
>>>>
>>>> Signed-off-by: Adrian Chadd <adrian@FreeBSD.org>
>>>
>>> [...]
>>>
>>>> --- a/drivers/net/wireless/ath/ath10k/core.c
>>>> +++ b/drivers/net/wireless/ath/ath10k/core.c
>>>> @@ -2444,6 +2444,8 @@ struct ath10k *ath10k_core_create(size_t
>>>> priv_size, struct device *dev,
>>>>     ar->hw_rev = hw_rev;
>>>>     ar->hif.ops = hif_ops;
>>>>     ar->hif.bus = bus;
>>>> +   ar->debug_mask = ath10k_debug_mask;
>>>> +   ar->trace_debug_mask = ath10k_debug_mask;
>>>
>>> Until now tracing has been always enabled, irrespective what debug_mask
>>> has contained. Now you are changing that and by default log messages are
>>> not delivered through tracing until user enables them. So I think to
>>> keep the old behaviour trace_debug_mask should be ATH10K_DBG_ANY
>>> (0xffffffff) by default and the user can modify the mask per device via
>>> the debugfs file.
>>>
>>> But is it really needed to be able to filter trace messages? debug_mask
>>> I understand, but not sure about trace_debug_mask.
>>
>> FWIW, in brcmfmac I decided not to filter trace messages. The overhead
>> is relatively small and if needed you can pass filter expressions with
>> trace-cmd record.
>
> I also think that this is how it should work. For example, if you have
> tracing enabled in wpasupplicant/hostapd with the command below you can
> get a lot of information in one file with relatively little overhead:
>
> trace-cmd record -e mac80211 -e cfg80211 -e ath10k
>
> But if user is forced to use debugfs to enable ath10k tracing that is
> quite a step backwards.
>

I agree that is how it should work, but i can't see how you do it on
ath10k without the debug overhead always being there.

If there's a way to know /a/ trace watch is going on from the kernel
then it could be flipped on for that, but i didn't see any obvious
"trace enable" / "trace disable" hooks.



-a

WARNING: multiple messages have this Message-ID (diff)
From: Adrian Chadd <adrian@freebsd.org>
To: Kalle Valo <kvalo@qca.qualcomm.com>
Cc: Arend van Spriel <arend.vanspriel@broadcom.com>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"ath10k@lists.infradead.org" <ath10k@lists.infradead.org>
Subject: Re: [PATCH] ath10k: add configurable debugging.
Date: Thu, 1 Jun 2017 06:33:38 -0700	[thread overview]
Message-ID: <CAJ-VmokfLW6RZyKZ4cKR7+cbTxwwa4aUZPm88Djv17Bo-oZ-0A@mail.gmail.com> (raw)
In-Reply-To: <87r2z3lu3j.fsf@kamboji.qca.qualcomm.com>

On 1 June 2017 at 06:24, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>
>> On 31-05-17 14:16, Kalle Valo wrote:
>>> Adrian Chadd <adrian@freebsd.org> writes:
>>>
>>>> This adds a few configurable debugging options:
>>>>
>>>> * driver debugging and tracing is now configurable per device
>>>> * driver debugging and tracing is now configurable at runtime
>>>> * the debugging / tracing is not run at all (besides a mask check)
>>>>   unless the specific debugging bitmap field is configured.
>>>>
>>>> Signed-off-by: Adrian Chadd <adrian@FreeBSD.org>
>>>
>>> [...]
>>>
>>>> --- a/drivers/net/wireless/ath/ath10k/core.c
>>>> +++ b/drivers/net/wireless/ath/ath10k/core.c
>>>> @@ -2444,6 +2444,8 @@ struct ath10k *ath10k_core_create(size_t
>>>> priv_size, struct device *dev,
>>>>     ar->hw_rev = hw_rev;
>>>>     ar->hif.ops = hif_ops;
>>>>     ar->hif.bus = bus;
>>>> +   ar->debug_mask = ath10k_debug_mask;
>>>> +   ar->trace_debug_mask = ath10k_debug_mask;
>>>
>>> Until now tracing has been always enabled, irrespective what debug_mask
>>> has contained. Now you are changing that and by default log messages are
>>> not delivered through tracing until user enables them. So I think to
>>> keep the old behaviour trace_debug_mask should be ATH10K_DBG_ANY
>>> (0xffffffff) by default and the user can modify the mask per device via
>>> the debugfs file.
>>>
>>> But is it really needed to be able to filter trace messages? debug_mask
>>> I understand, but not sure about trace_debug_mask.
>>
>> FWIW, in brcmfmac I decided not to filter trace messages. The overhead
>> is relatively small and if needed you can pass filter expressions with
>> trace-cmd record.
>
> I also think that this is how it should work. For example, if you have
> tracing enabled in wpasupplicant/hostapd with the command below you can
> get a lot of information in one file with relatively little overhead:
>
> trace-cmd record -e mac80211 -e cfg80211 -e ath10k
>
> But if user is forced to use debugfs to enable ath10k tracing that is
> quite a step backwards.
>

I agree that is how it should work, but i can't see how you do it on
ath10k without the debug overhead always being there.

If there's a way to know /a/ trace watch is going on from the kernel
then it could be flipped on for that, but i didn't see any obvious
"trace enable" / "trace disable" hooks.



-a

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

  reply	other threads:[~2017-06-01 13:33 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-10 21:19 [PATCH] ath10k: add configurable debugging Adrian Chadd
2017-05-10 21:19 ` Adrian Chadd
2017-05-19  9:51 ` Kalle Valo
2017-05-19  9:51   ` Kalle Valo
2017-05-31 11:53 ` Kalle Valo
2017-05-31 11:53   ` Kalle Valo
2017-05-31 12:16 ` Kalle Valo
2017-05-31 12:16   ` Kalle Valo
2017-05-31 20:20   ` Arend van Spriel
2017-05-31 20:20     ` Arend van Spriel
2017-05-31 20:23     ` Adrian Chadd
2017-05-31 20:23       ` Adrian Chadd
2017-05-31 21:28       ` Arend van Spriel
2017-05-31 21:28         ` Arend van Spriel
2017-05-31 21:32         ` Adrian Chadd
2017-05-31 21:32           ` Adrian Chadd
2017-06-01 13:34       ` Kalle Valo
2017-06-01 13:34         ` Kalle Valo
2017-06-01 13:24     ` Kalle Valo
2017-06-01 13:24       ` Kalle Valo
2017-06-01 13:33       ` Adrian Chadd [this message]
2017-06-01 13:33         ` Adrian Chadd
  -- strict thread matches above, loose matches on Subject: below --
2017-05-10 16:25 Adrian Chadd
2017-05-10 16:25 ` Adrian Chadd
2017-05-10 16:44 ` Steve deRosier
2017-05-10 16:44   ` Steve deRosier
2017-05-10 16:50   ` Adrian Chadd
2017-05-10 16:50     ` Adrian Chadd
2017-05-19  9:47 ` Kalle Valo
2017-05-19  9:47   ` Kalle Valo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAJ-VmokfLW6RZyKZ4cKR7+cbTxwwa4aUZPm88Djv17Bo-oZ-0A@mail.gmail.com \
    --to=adrian@freebsd.org \
    --cc=arend.vanspriel@broadcom.com \
    --cc=ath10k@lists.infradead.org \
    --cc=kvalo@qca.qualcomm.com \
    --cc=linux-wireless@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.