Date:   Fri, 11 Aug 2023 18:14:33 +0800
From:   Yicong Yang <yangyicong@...wei.com>
To:     Liang Li <liliang6@...il.cn>
CC:     <yangyicong@...ilicon.com>, Barry Song <21cnbao@...il.com>,
        <will@...nel.org>, <mark.rutland@....com>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>, <jonathan.cameron@...wei.com>,
        <linuxarm@...wei.com>
Subject: Re: [PATCH] perf/smmuv3: Add platform id table for module auto
 loading

On 2023/8/9 20:11, Liang Li wrote:
> Hi Yicong,
> 
> Thanks for your reply,
> 
> On 2023-08-09 14:31, Yicong Yang <yangyicong@...wei.com> wrote:
>> Hi Barry, Liang,
>>
>> On 2023/8/9 13:47, Barry Song wrote:
>>> On Wed, Aug 9, 2023 at 1:01 PM Liang Li <liliang6@...il.cn> wrote:
>>>>
>>>> On 2023-08-07 20:22, Yicong Yang <yangyicong@...wei.com> wrote:
>>>>> From: Yicong Yang <yangyicong@...ilicon.com>
>>>>>
>>>>> On ACPI based system the device is probed by the name directly. If the
>>>>> driver is configured as module it can only be loaded manually. Add the
>>>>> platform id table as well as the module alias then the driver will be
>>>>> loaded automatically by the udev or others once the device added.
>>>>>
>>>>
>>>> Please consider revise the long log to clearly express the purpose of the
>>>> changes in this patch:
>>>>
>>>> - What's the exact issue the patch is addressing
>>>> - Why the changes in this patch can fix the issue or make something working
>>>> - Consider impact of the changes introduced by this patch
>>>>
>>>> These info may help reviewers and maintainers .. and yourself on code merge.
>>>
>>> years ago, i found a good doc regarding this,
>>> https://wiki.archlinux.org/title/Modalias
>>>
>>> guess it is because /lib/modules/$(uname -r)/modules.alias fails to contain smmu
>>> driver without the MODULE_DEVICE_TABLE, isn't it, yicong？
>>
>> Yes I think it's the reason. I didn't find summary in kernel docs for the modalias
>> as well as the uevent mechanism. Arch wiki has a well illustration for the modalias
>> and suse[1] describes how this is used by the udev for module auto loading.
>>
>> For my case I'm using a ACPI based arm64 server and after booting the arm_smmuv3_pmu.ko
>> is not auto loaded by the udevd since we aren't providing this information. In order
>> to support this we need to provide this MODULE_DEVICE_TABLE() when the smmu pmu added
>> as a platform device, then the userspace udev can know which module to load after the
>> device is added.
>>
> 
> Then what's the purpose of the added '.id_table = ...' line in the previous
> patch ?
> <We lost the patch context in this thread.>
> 
> Based on above clarification, the updated DEVICE_TABLE would update modalias
> as expected, right ?

ok, it's lack of illustration in the commit. If we're going to use MODULE_DEVICE_TABLE we need
a platform id table. So I add it and I found it weired if we have a id table but not use it for
probing, so I also initialize .id_table.

I found there's also an another way to implement this by used MODULE_ALIAS(), and no need to add
an id table. Maybe this way is less controversial.

diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c
index 25a269d431e4..4c32b6dbfe76 100644
--- a/drivers/perf/arm_smmuv3_pmu.c
+++ b/drivers/perf/arm_smmuv3_pmu.c
@@ -984,6 +984,7 @@ static void __exit arm_smmu_pmu_exit(void)

 module_exit(arm_smmu_pmu_exit);

+MODULE_ALIAS("platform:arm-smmu-v3-pmcg");
 MODULE_DESCRIPTION("PMU driver for ARM SMMUv3 Performance Monitors Extension");
 MODULE_AUTHOR("Neil Leeder <nleeder@...eaurora.org>");
 MODULE_AUTHOR("Shameer Kolothum <shameerali.kolothum.thodi@...wei.com>");

Powered by blists - more mailing lists