[PATCH] perf/smmuv3: Add platform id table for module auto loading

[PATCH] perf/smmuv3: Add platform id table for module auto loading

[Yicong Yang]

From: Yicong Yang <yangyicong@hisilicon.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.

Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
 drivers/perf/arm_smmuv3_pmu.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c
index 25a269d431e4..f27c5f585524 100644
--- a/drivers/perf/arm_smmuv3_pmu.c
+++ b/drivers/perf/arm_smmuv3_pmu.c
@@ -946,7 +946,14 @@ static const struct of_device_id smmu_pmu_of_match[] = {
 MODULE_DEVICE_TABLE(of, smmu_pmu_of_match);
 #endif
 
+static const struct platform_device_id smmu_pmu_platform_match[] = {
+	{ "arm-smmu-v3-pmcg", 0 },
+	{}
+};
+MODULE_DEVICE_TABLE(platform, smmu_pmu_platform_match);
+
 static struct platform_driver smmu_pmu_driver = {
+	.id_table = smmu_pmu_platform_match,
 	.driver = {
 		.name = "arm-smmu-v3-pmcg",
 		.of_match_table = of_match_ptr(smmu_pmu_of_match),
-- 
2.24.0


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

Re: [PATCH] perf/smmuv3: Add platform id table for module auto loading

[Liang Li]

On 2023-08-07 20:22, Yicong Yang <yangyicong@huawei.com> wrote:
> From: Yicong Yang <yangyicong@hisilicon.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.

Regards.


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

Re: [PATCH] perf/smmuv3: Add platform id table for module auto loading

[Barry Song]

On Wed, Aug 9, 2023 at 1:01 PM Liang Li <liliang6@email.cn> wrote:
>
> On 2023-08-07 20:22, Yicong Yang <yangyicong@huawei.com> wrote:
> > From: Yicong Yang <yangyicong@hisilicon.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？

>
> Regards.
>

Thanks
Barry

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

Re: [PATCH] perf/smmuv3: Add platform id table for module auto loading

[Yicong Yang]

Hi Barry, Liang,

On 2023/8/9 13:47, Barry Song wrote:
> On Wed, Aug 9, 2023 at 1:01 PM Liang Li <liliang6@email.cn> wrote:
>>
>> On 2023-08-07 20:22, Yicong Yang <yangyicong@huawei.com> wrote:
>>> From: Yicong Yang <yangyicong@hisilicon.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.

[1] https://documentation.suse.com/sles/15-SP1/html/SLES-all/cha-udev.html#sec-udev-drivers

Thanks.

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

Re: [PATCH] perf/smmuv3: Add platform id table for module auto loading

[Liang Li]

Hi Yicong,

Thanks for your reply,

On 2023-08-09 14:31, Yicong Yang <yangyicong@huawei.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@email.cn> wrote:
> >>
> >> On 2023-08-07 20:22, Yicong Yang <yangyicong@huawei.com> wrote:
> >>> From: Yicong Yang <yangyicong@hisilicon.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 ?

> [1] https://documentation.suse.com/sles/15-SP1/html/SLES-all/cha-udev.html#sec-udev-drivers
> 
> Thanks.

Regards.
Liang Li


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

Re: [PATCH] perf/smmuv3: Add platform id table for module auto loading

[Yicong Yang]

On 2023/8/9 20:11, Liang Li wrote:
> Hi Yicong,
> 
> Thanks for your reply,
> 
> On 2023-08-09 14:31, Yicong Yang <yangyicong@huawei.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@email.cn> wrote:
>>>>
>>>> On 2023-08-07 20:22, Yicong Yang <yangyicong@huawei.com> wrote:
>>>>> From: Yicong Yang <yangyicong@hisilicon.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@codeaurora.org>");
 MODULE_AUTHOR("Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>");


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

Re: [PATCH] perf/smmuv3: Add platform id table for module auto loading

[Barry Song]

On Fri, Aug 11, 2023 at 6:14 PM Yicong Yang <yangyicong@huawei.com> wrote:
>
> On 2023/8/9 20:11, Liang Li wrote:
> > Hi Yicong,
> >
> > Thanks for your reply,
> >
> > On 2023-08-09 14:31, Yicong Yang <yangyicong@huawei.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@email.cn> wrote:
> >>>>
> >>>> On 2023-08-07 20:22, Yicong Yang <yangyicong@huawei.com> wrote:
> >>>>> From: Yicong Yang <yangyicong@hisilicon.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.

right, how is your driver matching with your device?

acpi_driver_match_device()  or strcmp(pdev->name, drv->name) == 0 ?

static int platform_match(struct device *dev, struct device_driver *drv)
{
        struct platform_device *pdev = to_platform_device(dev);
        struct platform_driver *pdrv = to_platform_driver(drv);

        /* When driver_override is set, only bind to the matching driver */
        if (pdev->driver_override)
                return !strcmp(pdev->driver_override, drv->name);

        /* Attempt an OF style match first */
        if (of_driver_match_device(dev, drv))
                return 1;

        /* Then try ACPI style match */
        if (acpi_driver_match_device(dev, drv))
                return 1;

        /* Then try to match against the id table */
        if (pdrv->id_table)
                return platform_match_id(pdrv->id_table, pdev) != NULL;

        /* fall-back to driver name match */
        return (strcmp(pdev->name, drv->name) == 0);
}

In both cases, it seems we don't need id_table. id_table can support
two or above device names, so that one driver can support
multiple device IDs usually with different specific device data.

>
> 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@codeaurora.org>");
>  MODULE_AUTHOR("Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>");
>

Thanks
Barry

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

Re: [PATCH] perf/smmuv3: Add platform id table for module auto loading

[Barry Song]

On Sat, Aug 12, 2023 at 1:24 PM Barry Song <21cnbao@gmail.com> wrote:
>
> On Fri, Aug 11, 2023 at 6:14 PM Yicong Yang <yangyicong@huawei.com> wrote:
> >
> > On 2023/8/9 20:11, Liang Li wrote:
> > > Hi Yicong,
> > >
> > > Thanks for your reply,
> > >
> > > On 2023-08-09 14:31, Yicong Yang <yangyicong@huawei.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@email.cn> wrote:
> > >>>>
> > >>>> On 2023-08-07 20:22, Yicong Yang <yangyicong@huawei.com> wrote:
> > >>>>> From: Yicong Yang <yangyicong@hisilicon.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.
>
> right, how is your driver matching with your device?
>
> acpi_driver_match_device()  or strcmp(pdev->name, drv->name) == 0 ?

btw, those drivers supporting acpi probe usually have a acpi table

#if IS_ENABLED(CONFIG_ACPI)
static const struct acpi_device_id hidma_acpi_ids[] = {
{"QCOM8061"},
{"QCOM8062", HIDMA_MSI_CAP},
{"QCOM8063", (HIDMA_MSI_CAP | HIDMA_IDENTITY_CAP)},
{},
};
MODULE_DEVICE_TABLE(acpi, hidma_acpi_ids);
#endif

>
> static int platform_match(struct device *dev, struct device_driver *drv)
> {
>         struct platform_device *pdev = to_platform_device(dev);
>         struct platform_driver *pdrv = to_platform_driver(drv);
>
>         /* When driver_override is set, only bind to the matching driver */
>         if (pdev->driver_override)
>                 return !strcmp(pdev->driver_override, drv->name);
>
>         /* Attempt an OF style match first */
>         if (of_driver_match_device(dev, drv))
>                 return 1;
>
>         /* Then try ACPI style match */
>         if (acpi_driver_match_device(dev, drv))
>                 return 1;
>
>         /* Then try to match against the id table */
>         if (pdrv->id_table)
>                 return platform_match_id(pdrv->id_table, pdev) != NULL;
>
>         /* fall-back to driver name match */
>         return (strcmp(pdev->name, drv->name) == 0);
> }
>
> In both cases, it seems we don't need id_table. id_table can support
> two or above device names, so that one driver can support
> multiple device IDs usually with different specific device data.
>
> >
> > 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@codeaurora.org>");
> >  MODULE_AUTHOR("Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>");
> >
>
> Thanks
> Barry

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

Re: [PATCH] perf/smmuv3: Add platform id table for module auto loading

[Yicong Yang]

On 2023/8/12 13:31, Barry Song wrote:
> On Sat, Aug 12, 2023 at 1:24 PM Barry Song <21cnbao@gmail.com> wrote:
>>
>> On Fri, Aug 11, 2023 at 6:14 PM Yicong Yang <yangyicong@huawei.com> wrote:
>>>
>>> On 2023/8/9 20:11, Liang Li wrote:
>>>> Hi Yicong,
>>>>
>>>> Thanks for your reply,
>>>>
>>>> On 2023-08-09 14:31, Yicong Yang <yangyicong@huawei.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@email.cn> wrote:
>>>>>>>
>>>>>>> On 2023-08-07 20:22, Yicong Yang <yangyicong@huawei.com> wrote:
>>>>>>>> From: Yicong Yang <yangyicong@hisilicon.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.
>>
>> right, how is your driver matching with your device?
>>
>> acpi_driver_match_device()  or strcmp(pdev->name, drv->name) == 0 ?
> 

The latter. The smmu pmu platform device is created when parsing the IORT so probed by matching the
device and driver name directly.

> btw, those drivers supporting acpi probe usually have a acpi table
> 
> #if IS_ENABLED(CONFIG_ACPI)
> static const struct acpi_device_id hidma_acpi_ids[] = {
> {"QCOM8061"},
> {"QCOM8062", HIDMA_MSI_CAP},
> {"QCOM8063", (HIDMA_MSI_CAP | HIDMA_IDENTITY_CAP)},
> {},
> };
> MODULE_DEVICE_TABLE(acpi, hidma_acpi_ids);
> #endif
> 
>>
>> static int platform_match(struct device *dev, struct device_driver *drv)
>> {
>>         struct platform_device *pdev = to_platform_device(dev);
>>         struct platform_driver *pdrv = to_platform_driver(drv);
>>
>>         /* When driver_override is set, only bind to the matching driver */
>>         if (pdev->driver_override)
>>                 return !strcmp(pdev->driver_override, drv->name);
>>
>>         /* Attempt an OF style match first */
>>         if (of_driver_match_device(dev, drv))
>>                 return 1;
>>
>>         /* Then try ACPI style match */
>>         if (acpi_driver_match_device(dev, drv))
>>                 return 1;
>>
>>         /* Then try to match against the id table */
>>         if (pdrv->id_table)
>>                 return platform_match_id(pdrv->id_table, pdev) != NULL;
>>
>>         /* fall-back to driver name match */
>>         return (strcmp(pdev->name, drv->name) == 0);
>> }
>>
>> In both cases, it seems we don't need id_table. id_table can support
>> two or above device names, so that one driver can support
>> multiple device IDs usually with different specific device data.
>>

yes I think you're right. For my case use MODULE_ALIAS is enough. Will respin
a v2.

Thanks!

>>>
>>> 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@codeaurora.org>");
>>>  MODULE_AUTHOR("Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>");
>>>
>>
>> Thanks
>> Barry
> .
> 

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