Re: [PATCH v7 25/25] coresight: allow the coresight core driver to be built as a module

From: Suzuki K Poulose <suzuki.poulose@arm.com>
To: tingwei@codeaurora.org, mathieu.poirier@linaro.org,
	alexander.shishkin@linux.intel.com, mike.leach@linaro.org
Cc: tsoni@codeaurora.org, saiprakash.ranjan@codeaurora.org,
	kim.phillips@arm.com, jinlmao@codeaurora.org,
	gregkh@linuxfoundation.org, coresight@lists.linaro.org,
	rdunlap@infradead.org, ykaukab@suse.de, linux@armlinux.org.uk,
	leo.yan@linaro.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v7 25/25] coresight: allow the coresight core driver to be built as a module
Date: Thu, 6 Aug 2020 17:33:22 +0100	[thread overview]
Message-ID: <aad73c7c-1e29-a586-e691-1b926fbbbf14@arm.com> (raw)
In-Reply-To: <1f791302-4905-0f2c-6857-fab957e8d811@arm.com>

On 08/05/2020 05:29 PM, Suzuki K Poulose wrote:
> On 08/05/2020 03:54 AM, Tingwei Zhang wrote:
>> Enhance coresight developer's efficiency to debug coresight drivers.
>> - Kconfig becomes a tristate, to allow =m
>> - append -core to source file name to allow module to
>>    be called coresight by the Makefile
>> - modules can have only one init/exit, so we add the etm_perf
>>    register/unregister function calls to the core init/exit
>>    functions.
>> - add a MODULE_DEVICE_TABLE for autoloading on boot
>>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Leo Yan <leo.yan@linaro.org>
>> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> Cc: Randy Dunlap <rdunlap@infradead.org>
>> Cc: Suzuki K Poulose <Suzuki.Poulose@arm.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Russell King <linux@armlinux.org.uk>
>> Signed-off-by: Kim Phillips <kim.phillips@arm.com>
>> Signed-off-by: Tingwei Zhang <tingwei@codeaurora.org>
>> Tested-by: Mike Leach <mike.leach@linaro.org>
>> ---
>>   drivers/hwtracing/coresight/Kconfig           |  5 ++-
>>   drivers/hwtracing/coresight/Makefile          |  5 ++-
>>   .../{coresight.c => coresight-core.c}         | 42 ++++++++++++++-----
>>   .../hwtracing/coresight/coresight-etm-perf.c  |  8 +++-
>>   .../hwtracing/coresight/coresight-etm-perf.h  |  3 ++
>>   5 files changed, 48 insertions(+), 15 deletions(-)
>>   rename drivers/hwtracing/coresight/{coresight.c => coresight-core.c} 
>> (98%)
> 
> Personally, I would like to rename this to core.c dropping the
> "coresight-" prefix here (now that we have to do a rename). And we
> should do that ideally for all the other files (but not proposing
> it to be part of this series, and could be something that we could
> pursue if everyone agrees to it).
> 
> We are inside the coresight directory anyways and having a prefix
> doesn't help with anything.
> 
> The patch as such looks good to me.
> 
> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

On a second look, I believe for the sake of completion, we
should set the "owner" of the etm, now that we are a module.
The question is, which one should that be. It could be the
"coresight" or the "coresight-etm{3,4}x".

I believe the "coresight" is the better choice.

Cheers
Suzuki

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel