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

[Robin Murphy]

On 2020-08-06 17:33, Suzuki K Poulose wrote:
> 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 at linaro.org>
>>> Cc: Leo Yan <leo.yan at linaro.org>
>>> Cc: Alexander Shishkin <alexander.shishkin at linux.intel.com>
>>> Cc: Randy Dunlap <rdunlap at infradead.org>
>>> Cc: Suzuki K Poulose <Suzuki.Poulose at arm.com>
>>> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
>>> Cc: Russell King <linux at armlinux.org.uk>
>>> Signed-off-by: Kim Phillips <kim.phillips at arm.com>
>>> Signed-off-by: Tingwei Zhang <tingwei at codeaurora.org>
>>> Tested-by: Mike Leach <mike.leach at 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 at 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.

If you mean pmu->owner, you shouldn't really have much of a choice - it 
should be the module containing the actual PMU callbacks, such that they 
can't suddenly disappear while the PMU is in use. Allowing perf to take 
a reference to some other module and not actually protect itself would 
not be good. It should be pretty rare that the correct owner is anything 
other than THIS_MODULE ;)

Hopefully the dependencies are such that the core module automatically 
holds its own reference to the individual ETM driver module(s) by the 
time it registers the PMU.

Robin.