Re: [PATCH v2] ath: Add support for tracing

[Kalle Valo <kvalo@qca.qualcomm.com>]

Sujith Manoharan <sujith@msujith.org> writes:

> Kalle Valo wrote:
>> You mean ath_printk() & friends? But that doesn't require tracing code
>> to be in ath.ko as well, right? If I understood correctly, trace.c could
>> be under ath9k directory and the kconfig option could be
>> ATH9K_TRACEPOINTS.
>> 
>> I think it's just misleading and confusing for the user to call it
>> "Atheros wireless tracing" when it only affects ath9k. It's easier to
>> understand if each driver has it's own "tracing" kconfig option.
>
> We have CONFIG_ATH_DEBUG, which is used by ath9k and ath9k_htc.
> I think it is okay to have CONFIG_ATH_TRACEPOINTS, which can be
> used by ath9k/ath9k_htc too.

In my opinion that just creates even a bigger mess.

> The original motive of ath.ko was to have a common module with
> debugging facilities that can be shared by Atheros drivers. But,
> each driver has ended up reinventing things.

The current debug printing code in ath10k is something like 100 lines, I
don't see the point of trying to make that common with all ath* drivers.

In my opinion ath.ko should only have code which used at least two
different drivers (and I consider ath9k.ko and ath9k_htc.ko as one
driver). So the right thing here would be to actually move all debugging
code from ath.ko to ath9k, as it's the only user anyway.

> Since it is mentioned in the help text that ath9k is the only driver
> making use of ATH_DEBUG/ATH_TRACEPOINTS, I don't think it is
> confusing.

To me it is. But it's a mess already so I guess I'm worrying nothing.

-- 
Kalle Valo