2024-06-04 04:49:25

by Jeff Johnson

[permalink] [raw]
Subject: [PATCH] cxl: add missing MODULE_DESCRIPTION() macros

make allmodconfig && make W=1 C=1 reports:
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/core/cxl_core.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_pci.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_mem.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_acpi.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_pmem.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_port.o

Add the missing invocations of the MODULE_DESCRIPTION() macro.

Signed-off-by: Jeff Johnson <[email protected]>
---
drivers/cxl/acpi.c | 1 +
drivers/cxl/core/port.c | 1 +
drivers/cxl/mem.c | 1 +
drivers/cxl/pci.c | 1 +
drivers/cxl/pmem.c | 1 +
drivers/cxl/port.c | 1 +
6 files changed, 6 insertions(+)

diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index 571069863c62..e51315ea4a6a 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -921,6 +921,7 @@ static void __exit cxl_acpi_exit(void)
/* load before dax_hmem sees 'Soft Reserved' CXL ranges */
subsys_initcall(cxl_acpi_init);
module_exit(cxl_acpi_exit);
+MODULE_DESCRIPTION("CXL ACPI: Platform Support");
MODULE_LICENSE("GPL v2");
MODULE_IMPORT_NS(CXL);
MODULE_IMPORT_NS(ACPI);
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 887ed6e358fb..ccaa00cd0321 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -2356,5 +2356,6 @@ static void cxl_core_exit(void)

subsys_initcall(cxl_core_init);
module_exit(cxl_core_exit);
+MODULE_DESCRIPTION("CXL (Compute Express Link) Devices Support");
MODULE_LICENSE("GPL v2");
MODULE_IMPORT_NS(CXL);
diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
index 0c79d9ce877c..1afb0e78082b 100644
--- a/drivers/cxl/mem.c
+++ b/drivers/cxl/mem.c
@@ -252,6 +252,7 @@ static struct cxl_driver cxl_mem_driver = {

module_cxl_driver(cxl_mem_driver);

+MODULE_DESCRIPTION("CXL: Memory Expansion");
MODULE_LICENSE("GPL v2");
MODULE_IMPORT_NS(CXL);
MODULE_ALIAS_CXL(CXL_DEVICE_MEMORY_EXPANDER);
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index e53646e9f2fb..2c17fcb1b4ee 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -1066,5 +1066,6 @@ static void __exit cxl_pci_driver_exit(void)

module_init(cxl_pci_driver_init);
module_exit(cxl_pci_driver_exit);
+MODULE_DESCRIPTION("CXL PCI manageability");
MODULE_LICENSE("GPL v2");
MODULE_IMPORT_NS(CXL);
diff --git a/drivers/cxl/pmem.c b/drivers/cxl/pmem.c
index 2ecdaee63021..4ef93da22335 100644
--- a/drivers/cxl/pmem.c
+++ b/drivers/cxl/pmem.c
@@ -453,6 +453,7 @@ static __exit void cxl_pmem_exit(void)
cxl_driver_unregister(&cxl_nvdimm_bridge_driver);
}

+MODULE_DESCRIPTION("CXL PMEM: Persistent Memory Support");
MODULE_LICENSE("GPL v2");
module_init(cxl_pmem_init);
module_exit(cxl_pmem_exit);
diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index 97c21566677a..5ceff1df60db 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -209,6 +209,7 @@ static struct cxl_driver cxl_port_driver = {
};

module_cxl_driver(cxl_port_driver);
+MODULE_DESCRIPTION("CXL Port Support");
MODULE_LICENSE("GPL v2");
MODULE_IMPORT_NS(CXL);
MODULE_ALIAS_CXL(CXL_DEVICE_PORT);

---
base-commit: a693b9c95abd4947c2d06e05733de5d470ab6586
change-id: 20240603-md-drivers-cxl-85ac807b9618



2024-06-04 16:07:27

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH] cxl: add missing MODULE_DESCRIPTION() macros

On Mon, 3 Jun 2024 21:48:53 -0700
Jeff Johnson <[email protected]> wrote:

> make allmodconfig && make W=1 C=1 reports:
> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/core/cxl_core.o
> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_pci.o
> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_mem.o
> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_acpi.o
> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_pmem.o
> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_port.o
>
> Add the missing invocations of the MODULE_DESCRIPTION() macro.
>
> Signed-off-by: Jeff Johnson <[email protected]>

This has been irritating me as well. Need to do
drivers/perf/cxl_pmu.c at somepoint as well but given that goes through
a different maintainer makes sense to do separately.

Only comment I have is that we should probably strive for more consistency
than you currently have. Always expand CXL or never do, use
colons consistently, use Support everywhere or nowhere.



> ---
> drivers/cxl/acpi.c | 1 +
> drivers/cxl/core/port.c | 1 +
> drivers/cxl/mem.c | 1 +
> drivers/cxl/pci.c | 1 +
> drivers/cxl/pmem.c | 1 +
> drivers/cxl/port.c | 1 +
> 6 files changed, 6 insertions(+)
>
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 571069863c62..e51315ea4a6a 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -921,6 +921,7 @@ static void __exit cxl_acpi_exit(void)
> /* load before dax_hmem sees 'Soft Reserved' CXL ranges */
> subsys_initcall(cxl_acpi_init);
> module_exit(cxl_acpi_exit);
> +MODULE_DESCRIPTION("CXL ACPI: Platform Support");
> MODULE_LICENSE("GPL v2");
> MODULE_IMPORT_NS(CXL);
> MODULE_IMPORT_NS(ACPI);
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 887ed6e358fb..ccaa00cd0321 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -2356,5 +2356,6 @@ static void cxl_core_exit(void)
>
> subsys_initcall(cxl_core_init);
> module_exit(cxl_core_exit);
> +MODULE_DESCRIPTION("CXL (Compute Express Link) Devices Support");

Why the expanded version for this one?

I'm not sure Devices really makes sense here, particularly as it
likely a range of other driver will make some use of this core
functionality over time. Maybe "CXL core" is sufficient?

> MODULE_LICENSE("GPL v2");
> MODULE_IMPORT_NS(CXL);
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index 0c79d9ce877c..1afb0e78082b 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -252,6 +252,7 @@ static struct cxl_driver cxl_mem_driver = {
>
> module_cxl_driver(cxl_mem_driver);
>
> +MODULE_DESCRIPTION("CXL: Memory Expansion");

Why does this one get a colon? Also no Support at the end?

> MODULE_LICENSE("GPL v2");
> MODULE_IMPORT_NS(CXL);
> MODULE_ALIAS_CXL(CXL_DEVICE_MEMORY_EXPANDER);
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index e53646e9f2fb..2c17fcb1b4ee 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -1066,5 +1066,6 @@ static void __exit cxl_pci_driver_exit(void)
>
> module_init(cxl_pci_driver_init);
> module_exit(cxl_pci_driver_exit);
> +MODULE_DESCRIPTION("CXL PCI manageability");
> MODULE_LICENSE("GPL v2");
> MODULE_IMPORT_NS(CXL);
> diff --git a/drivers/cxl/pmem.c b/drivers/cxl/pmem.c
> index 2ecdaee63021..4ef93da22335 100644
> --- a/drivers/cxl/pmem.c
> +++ b/drivers/cxl/pmem.c
> @@ -453,6 +453,7 @@ static __exit void cxl_pmem_exit(void)
> cxl_driver_unregister(&cxl_nvdimm_bridge_driver);
> }
>
> +MODULE_DESCRIPTION("CXL PMEM: Persistent Memory Support");
> MODULE_LICENSE("GPL v2");
> module_init(cxl_pmem_init);
> module_exit(cxl_pmem_exit);
> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index 97c21566677a..5ceff1df60db 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -209,6 +209,7 @@ static struct cxl_driver cxl_port_driver = {
> };
>
> module_cxl_driver(cxl_port_driver);
> +MODULE_DESCRIPTION("CXL Port Support");
> MODULE_LICENSE("GPL v2");
> MODULE_IMPORT_NS(CXL);
> MODULE_ALIAS_CXL(CXL_DEVICE_PORT);
>
> ---
> base-commit: a693b9c95abd4947c2d06e05733de5d470ab6586
> change-id: 20240603-md-drivers-cxl-85ac807b9618
>


2024-06-04 20:22:23

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH] cxl: add missing MODULE_DESCRIPTION() macros

On 6/4/2024 9:04 AM, Jonathan Cameron wrote:
> On Mon, 3 Jun 2024 21:48:53 -0700
> Jeff Johnson <[email protected]> wrote:
>
>> make allmodconfig && make W=1 C=1 reports:
>> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/core/cxl_core.o
>> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_pci.o
>> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_mem.o
>> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_acpi.o
>> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_pmem.o
>> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_port.o
>>
>> Add the missing invocations of the MODULE_DESCRIPTION() macro.
>>
>> Signed-off-by: Jeff Johnson <[email protected]>
>
> This has been irritating me as well. Need to do
> drivers/perf/cxl_pmu.c at somepoint as well but given that goes through
> a different maintainer makes sense to do separately.
>
> Only comment I have is that we should probably strive for more consistency
> than you currently have. Always expand CXL or never do, use
> colons consistently, use Support everywhere or nowhere.

I'm going through a bunch of these tree-wide, and usually just copy/paste
either from existing comments in the .c file or the description of any
associated Kconfig item.

>> ---
>> drivers/cxl/acpi.c | 1 +
>> drivers/cxl/core/port.c | 1 +
>> drivers/cxl/mem.c | 1 +
>> drivers/cxl/pci.c | 1 +
>> drivers/cxl/pmem.c | 1 +
>> drivers/cxl/port.c | 1 +
>> 6 files changed, 6 insertions(+)
>>
>> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
>> index 571069863c62..e51315ea4a6a 100644
>> --- a/drivers/cxl/acpi.c
>> +++ b/drivers/cxl/acpi.c
>> @@ -921,6 +921,7 @@ static void __exit cxl_acpi_exit(void)
>> /* load before dax_hmem sees 'Soft Reserved' CXL ranges */
>> subsys_initcall(cxl_acpi_init);
>> module_exit(cxl_acpi_exit);
>> +MODULE_DESCRIPTION("CXL ACPI: Platform Support");

From Kconfig:
config CXL_ACPI
tristate "CXL ACPI: Platform Support"

>> MODULE_LICENSE("GPL v2");
>> MODULE_IMPORT_NS(CXL);
>> MODULE_IMPORT_NS(ACPI);
>> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
>> index 887ed6e358fb..ccaa00cd0321 100644
>> --- a/drivers/cxl/core/port.c
>> +++ b/drivers/cxl/core/port.c
>> @@ -2356,5 +2356,6 @@ static void cxl_core_exit(void)
>>
>> subsys_initcall(cxl_core_init);
>> module_exit(cxl_core_exit);
>> +MODULE_DESCRIPTION("CXL (Compute Express Link) Devices Support");
>
> Why the expanded version for this one?
>
> I'm not sure Devices really makes sense here, particularly as it
> likely a range of other driver will make some use of this core
> functionality over time. Maybe "CXL core" is sufficient?
>

From Kconfig:
menuconfig CXL_BUS
tristate "CXL (Compute Express Link) Devices Support"

>> MODULE_LICENSE("GPL v2");
>> MODULE_IMPORT_NS(CXL);
>> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
>> index 0c79d9ce877c..1afb0e78082b 100644
>> --- a/drivers/cxl/mem.c
>> +++ b/drivers/cxl/mem.c
>> @@ -252,6 +252,7 @@ static struct cxl_driver cxl_mem_driver = {
>>
>> module_cxl_driver(cxl_mem_driver);
>>
>> +MODULE_DESCRIPTION("CXL: Memory Expansion");
>
> Why does this one get a colon? Also no Support at the end?

From Kconfig:
config CXL_MEM
tristate "CXL: Memory Expansion"

>
>> MODULE_LICENSE("GPL v2");
>> MODULE_IMPORT_NS(CXL);
>> MODULE_ALIAS_CXL(CXL_DEVICE_MEMORY_EXPANDER);
>> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
>> index e53646e9f2fb..2c17fcb1b4ee 100644
>> --- a/drivers/cxl/pci.c
>> +++ b/drivers/cxl/pci.c
>> @@ -1066,5 +1066,6 @@ static void __exit cxl_pci_driver_exit(void)
>>
>> module_init(cxl_pci_driver_init);
>> module_exit(cxl_pci_driver_exit);
>> +MODULE_DESCRIPTION("CXL PCI manageability");

Kconfig just has:
config CXL_PCI
tristate "PCI manageability"

I added CXL

>> MODULE_LICENSE("GPL v2");
>> MODULE_IMPORT_NS(CXL);
>> diff --git a/drivers/cxl/pmem.c b/drivers/cxl/pmem.c
>> index 2ecdaee63021..4ef93da22335 100644
>> --- a/drivers/cxl/pmem.c
>> +++ b/drivers/cxl/pmem.c
>> @@ -453,6 +453,7 @@ static __exit void cxl_pmem_exit(void)
>> cxl_driver_unregister(&cxl_nvdimm_bridge_driver);
>> }
>>
>> +MODULE_DESCRIPTION("CXL PMEM: Persistent Memory Support");

From Kconfig:
config CXL_PMEM
tristate "CXL PMEM: Persistent Memory Support"

>> MODULE_LICENSE("GPL v2");
>> module_init(cxl_pmem_init);
>> module_exit(cxl_pmem_exit);
>> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
>> index 97c21566677a..5ceff1df60db 100644
>> --- a/drivers/cxl/port.c
>> +++ b/drivers/cxl/port.c
>> @@ -209,6 +209,7 @@ static struct cxl_driver cxl_port_driver = {
>> };
>>
>> module_cxl_driver(cxl_port_driver);
>> +MODULE_DESCRIPTION("CXL Port Support");

This I just made up from the others since config CXL_PORT doesn't have a menu
description or help text and the .c file begins with:
* DOC: cxl port


>> MODULE_LICENSE("GPL v2");
>> MODULE_IMPORT_NS(CXL);
>> MODULE_ALIAS_CXL(CXL_DEVICE_PORT);

If you have specific edits you'd like me to make, I'm happy to make them.
I have no opinion on the content -- I just want to get rid of the warnings :)

/jeff


2024-06-06 14:30:15

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH] cxl: add missing MODULE_DESCRIPTION() macros

On Tue, 4 Jun 2024 13:21:52 -0700
Jeff Johnson <[email protected]> wrote:

> On 6/4/2024 9:04 AM, Jonathan Cameron wrote:
> > On Mon, 3 Jun 2024 21:48:53 -0700
> > Jeff Johnson <[email protected]> wrote:
> >
> >> make allmodconfig && make W=1 C=1 reports:
> >> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/core/cxl_core.o
> >> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_pci.o
> >> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_mem.o
> >> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_acpi.o
> >> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_pmem.o
> >> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_port.o
> >>
> >> Add the missing invocations of the MODULE_DESCRIPTION() macro.
> >>
> >> Signed-off-by: Jeff Johnson <[email protected]>
> >
> > This has been irritating me as well. Need to do
> > drivers/perf/cxl_pmu.c at somepoint as well but given that goes through
> > a different maintainer makes sense to do separately.
> >
> > Only comment I have is that we should probably strive for more consistency
> > than you currently have. Always expand CXL or never do, use
> > colons consistently, use Support everywhere or nowhere.
>
> I'm going through a bunch of these tree-wide, and usually just copy/paste
> either from existing comments in the .c file or the description of any
> associated Kconfig item.
>
> >> ---
> >> drivers/cxl/acpi.c | 1 +
> >> drivers/cxl/core/port.c | 1 +
> >> drivers/cxl/mem.c | 1 +
> >> drivers/cxl/pci.c | 1 +
> >> drivers/cxl/pmem.c | 1 +
> >> drivers/cxl/port.c | 1 +
> >> 6 files changed, 6 insertions(+)
> >>
> >> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> >> index 571069863c62..e51315ea4a6a 100644
> >> --- a/drivers/cxl/acpi.c
> >> +++ b/drivers/cxl/acpi.c
> >> @@ -921,6 +921,7 @@ static void __exit cxl_acpi_exit(void)
> >> /* load before dax_hmem sees 'Soft Reserved' CXL ranges */
> >> subsys_initcall(cxl_acpi_init);
> >> module_exit(cxl_acpi_exit);
> >> +MODULE_DESCRIPTION("CXL ACPI: Platform Support");
>
> From Kconfig:
> config CXL_ACPI
> tristate "CXL ACPI: Platform Support"
OK
>
> >> MODULE_LICENSE("GPL v2");
> >> MODULE_IMPORT_NS(CXL);
> >> MODULE_IMPORT_NS(ACPI);
> >> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> >> index 887ed6e358fb..ccaa00cd0321 100644
> >> --- a/drivers/cxl/core/port.c
> >> +++ b/drivers/cxl/core/port.c
> >> @@ -2356,5 +2356,6 @@ static void cxl_core_exit(void)
> >>
> >> subsys_initcall(cxl_core_init);
> >> module_exit(cxl_core_exit);
> >> +MODULE_DESCRIPTION("CXL (Compute Express Link) Devices Support");
> >
> > Why the expanded version for this one?
> >
> > I'm not sure Devices really makes sense here, particularly as it
> > likely a range of other driver will make some use of this core
> > functionality over time. Maybe "CXL core" is sufficient?
> >
>
> From Kconfig:
> menuconfig CXL_BUS
> tristate "CXL (Compute Express Link) Devices Support"

Understood, but that is expanded because it's the first use of
CXL in the make file. Here we have no ordering as across many
files and resulting modules.

"CXL: Core Compute Express Link support"

Would work I think.

>
> >> MODULE_LICENSE("GPL v2");
> >> MODULE_IMPORT_NS(CXL);
> >> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> >> index 0c79d9ce877c..1afb0e78082b 100644
> >> --- a/drivers/cxl/mem.c
> >> +++ b/drivers/cxl/mem.c
> >> @@ -252,6 +252,7 @@ static struct cxl_driver cxl_mem_driver = {
> >>
> >> module_cxl_driver(cxl_mem_driver);
> >>
> >> +MODULE_DESCRIPTION("CXL: Memory Expansion");
> >
> > Why does this one get a colon? Also no Support at the end?
>
> From Kconfig:
> config CXL_MEM
> tristate "CXL: Memory Expansion"

OK. Could add Support but then all code is supporting something,
so fine to leave it without.


>
> >
> >> MODULE_LICENSE("GPL v2");
> >> MODULE_IMPORT_NS(CXL);
> >> MODULE_ALIAS_CXL(CXL_DEVICE_MEMORY_EXPANDER);
> >> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> >> index e53646e9f2fb..2c17fcb1b4ee 100644
> >> --- a/drivers/cxl/pci.c
> >> +++ b/drivers/cxl/pci.c
> >> @@ -1066,5 +1066,6 @@ static void __exit cxl_pci_driver_exit(void)
> >>
> >> module_init(cxl_pci_driver_init);
> >> module_exit(cxl_pci_driver_exit);
> >> +MODULE_DESCRIPTION("CXL PCI manageability");
>
> Kconfig just has:
> config CXL_PCI
> tristate "PCI manageability"
>
> I added CXL

CXL: PCI manageability


>
> >> MODULE_LICENSE("GPL v2");
> >> MODULE_IMPORT_NS(CXL);
> >> diff --git a/drivers/cxl/pmem.c b/drivers/cxl/pmem.c
> >> index 2ecdaee63021..4ef93da22335 100644
> >> --- a/drivers/cxl/pmem.c
> >> +++ b/drivers/cxl/pmem.c
> >> @@ -453,6 +453,7 @@ static __exit void cxl_pmem_exit(void)
> >> cxl_driver_unregister(&cxl_nvdimm_bridge_driver);
> >> }
> >>
> >> +MODULE_DESCRIPTION("CXL PMEM: Persistent Memory Support");
>
> From Kconfig:
> config CXL_PMEM
> tristate "CXL PMEM: Persistent Memory Support"
OK
>
> >> MODULE_LICENSE("GPL v2");
> >> module_init(cxl_pmem_init);
> >> module_exit(cxl_pmem_exit);
> >> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> >> index 97c21566677a..5ceff1df60db 100644
> >> --- a/drivers/cxl/port.c
> >> +++ b/drivers/cxl/port.c
> >> @@ -209,6 +209,7 @@ static struct cxl_driver cxl_port_driver = {
> >> };
> >>
> >> module_cxl_driver(cxl_port_driver);
> >> +MODULE_DESCRIPTION("CXL Port Support");
>
> This I just made up from the others since config CXL_PORT doesn't have a menu
> description or help text and the .c file begins with:
> * DOC: cxl port

"CXL: Port Support"

Not that informative, but I can't immediately think of better text.

>
>
> >> MODULE_LICENSE("GPL v2");
> >> MODULE_IMPORT_NS(CXL);
> >> MODULE_ALIAS_CXL(CXL_DEVICE_PORT);
>
> If you have specific edits you'd like me to make, I'm happy to make them.
> I have no opinion on the content -- I just want to get rid of the warnings :)

With the suggestions above it would look more consistent I think.

Jonathan


>
> /jeff
>


2024-06-06 23:11:03

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH] cxl: add missing MODULE_DESCRIPTION() macros

On 6/6/2024 7:15 AM, Jonathan Cameron wrote:
> On Tue, 4 Jun 2024 13:21:52 -0700
> Jeff Johnson <[email protected]> wrote:
>
>> On 6/4/2024 9:04 AM, Jonathan Cameron wrote:
>>> On Mon, 3 Jun 2024 21:48:53 -0700
>>> Jeff Johnson <[email protected]> wrote:
>>>
>>>> make allmodconfig && make W=1 C=1 reports:
>>>> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/core/cxl_core.o
>>>> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_pci.o
>>>> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_mem.o
>>>> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_acpi.o
>>>> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_pmem.o
>>>> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/cxl/cxl_port.o
>>>>
>>>> Add the missing invocations of the MODULE_DESCRIPTION() macro.
>>>>
>>>> Signed-off-by: Jeff Johnson <[email protected]>
>>>
>>> This has been irritating me as well. Need to do
>>> drivers/perf/cxl_pmu.c at somepoint as well but given that goes through
>>> a different maintainer makes sense to do separately.
>>>
>>> Only comment I have is that we should probably strive for more consistency
>>> than you currently have. Always expand CXL or never do, use
>>> colons consistently, use Support everywhere or nowhere.
>>
>> I'm going through a bunch of these tree-wide, and usually just copy/paste
>> either from existing comments in the .c file or the description of any
>> associated Kconfig item.
>>
>>>> ---
>>>> drivers/cxl/acpi.c | 1 +
>>>> drivers/cxl/core/port.c | 1 +
>>>> drivers/cxl/mem.c | 1 +
>>>> drivers/cxl/pci.c | 1 +
>>>> drivers/cxl/pmem.c | 1 +
>>>> drivers/cxl/port.c | 1 +
>>>> 6 files changed, 6 insertions(+)
>>>>
>>>> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
>>>> index 571069863c62..e51315ea4a6a 100644
>>>> --- a/drivers/cxl/acpi.c
>>>> +++ b/drivers/cxl/acpi.c
>>>> @@ -921,6 +921,7 @@ static void __exit cxl_acpi_exit(void)
>>>> /* load before dax_hmem sees 'Soft Reserved' CXL ranges */
>>>> subsys_initcall(cxl_acpi_init);
>>>> module_exit(cxl_acpi_exit);
>>>> +MODULE_DESCRIPTION("CXL ACPI: Platform Support");
>>
>> From Kconfig:
>> config CXL_ACPI
>> tristate "CXL ACPI: Platform Support"
> OK
>>
>>>> MODULE_LICENSE("GPL v2");
>>>> MODULE_IMPORT_NS(CXL);
>>>> MODULE_IMPORT_NS(ACPI);
>>>> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
>>>> index 887ed6e358fb..ccaa00cd0321 100644
>>>> --- a/drivers/cxl/core/port.c
>>>> +++ b/drivers/cxl/core/port.c
>>>> @@ -2356,5 +2356,6 @@ static void cxl_core_exit(void)
>>>>
>>>> subsys_initcall(cxl_core_init);
>>>> module_exit(cxl_core_exit);
>>>> +MODULE_DESCRIPTION("CXL (Compute Express Link) Devices Support");
>>>
>>> Why the expanded version for this one?
>>>
>>> I'm not sure Devices really makes sense here, particularly as it
>>> likely a range of other driver will make some use of this core
>>> functionality over time. Maybe "CXL core" is sufficient?
>>>
>>
>> From Kconfig:
>> menuconfig CXL_BUS
>> tristate "CXL (Compute Express Link) Devices Support"
>
> Understood, but that is expanded because it's the first use of
> CXL in the make file. Here we have no ordering as across many
> files and resulting modules.
>
> "CXL: Core Compute Express Link support"
>
> Would work I think.
>
>>
>>>> MODULE_LICENSE("GPL v2");
>>>> MODULE_IMPORT_NS(CXL);
>>>> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
>>>> index 0c79d9ce877c..1afb0e78082b 100644
>>>> --- a/drivers/cxl/mem.c
>>>> +++ b/drivers/cxl/mem.c
>>>> @@ -252,6 +252,7 @@ static struct cxl_driver cxl_mem_driver = {
>>>>
>>>> module_cxl_driver(cxl_mem_driver);
>>>>
>>>> +MODULE_DESCRIPTION("CXL: Memory Expansion");
>>>
>>> Why does this one get a colon? Also no Support at the end?
>>
>> From Kconfig:
>> config CXL_MEM
>> tristate "CXL: Memory Expansion"
>
> OK. Could add Support but then all code is supporting something,
> so fine to leave it without.
>
>
>>
>>>
>>>> MODULE_LICENSE("GPL v2");
>>>> MODULE_IMPORT_NS(CXL);
>>>> MODULE_ALIAS_CXL(CXL_DEVICE_MEMORY_EXPANDER);
>>>> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
>>>> index e53646e9f2fb..2c17fcb1b4ee 100644
>>>> --- a/drivers/cxl/pci.c
>>>> +++ b/drivers/cxl/pci.c
>>>> @@ -1066,5 +1066,6 @@ static void __exit cxl_pci_driver_exit(void)
>>>>
>>>> module_init(cxl_pci_driver_init);
>>>> module_exit(cxl_pci_driver_exit);
>>>> +MODULE_DESCRIPTION("CXL PCI manageability");
>>
>> Kconfig just has:
>> config CXL_PCI
>> tristate "PCI manageability"
>>
>> I added CXL
>
> CXL: PCI manageability
>
>
>>
>>>> MODULE_LICENSE("GPL v2");
>>>> MODULE_IMPORT_NS(CXL);
>>>> diff --git a/drivers/cxl/pmem.c b/drivers/cxl/pmem.c
>>>> index 2ecdaee63021..4ef93da22335 100644
>>>> --- a/drivers/cxl/pmem.c
>>>> +++ b/drivers/cxl/pmem.c
>>>> @@ -453,6 +453,7 @@ static __exit void cxl_pmem_exit(void)
>>>> cxl_driver_unregister(&cxl_nvdimm_bridge_driver);
>>>> }
>>>>
>>>> +MODULE_DESCRIPTION("CXL PMEM: Persistent Memory Support");
>>
>> From Kconfig:
>> config CXL_PMEM
>> tristate "CXL PMEM: Persistent Memory Support"
> OK
>>
>>>> MODULE_LICENSE("GPL v2");
>>>> module_init(cxl_pmem_init);
>>>> module_exit(cxl_pmem_exit);
>>>> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
>>>> index 97c21566677a..5ceff1df60db 100644
>>>> --- a/drivers/cxl/port.c
>>>> +++ b/drivers/cxl/port.c
>>>> @@ -209,6 +209,7 @@ static struct cxl_driver cxl_port_driver = {
>>>> };
>>>>
>>>> module_cxl_driver(cxl_port_driver);
>>>> +MODULE_DESCRIPTION("CXL Port Support");
>>
>> This I just made up from the others since config CXL_PORT doesn't have a menu
>> description or help text and the .c file begins with:
>> * DOC: cxl port
>
> "CXL: Port Support"
>
> Not that informative, but I can't immediately think of better text.
>
>>
>>
>>>> MODULE_LICENSE("GPL v2");
>>>> MODULE_IMPORT_NS(CXL);
>>>> MODULE_ALIAS_CXL(CXL_DEVICE_PORT);
>>
>> If you have specific edits you'd like me to make, I'm happy to make them.
>> I have no opinion on the content -- I just want to get rid of the warnings :)
>
> With the suggestions above it would look more consistent I think.

Thanks for you specific suggestions.
I'll spin a v2 with those.

/jeff


2024-06-06 23:44:28

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH] cxl: add missing MODULE_DESCRIPTION() macros

Jeff Johnson wrote:
[..]
> >> This I just made up from the others since config CXL_PORT doesn't have a menu
> >> description or help text and the .c file begins with:
> >> * DOC: cxl port
> >
> > "CXL: Port Support"
> >
> > Not that informative, but I can't immediately think of better text.

How about "CXL: Port enumeration and services"

2024-06-07 08:30:24

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH] cxl: add missing MODULE_DESCRIPTION() macros

On Thu, 6 Jun 2024 16:42:44 -0700
Dan Williams <[email protected]> wrote:

> Jeff Johnson wrote:
> [..]
> > >> This I just made up from the others since config CXL_PORT doesn't have a menu
> > >> description or help text and the .c file begins with:
> > >> * DOC: cxl port
> > >
> > > "CXL: Port Support"
> > >
> > > Not that informative, but I can't immediately think of better text.
>
> How about "CXL: Port enumeration and services"
>

LGTM

2024-06-07 14:01:01

by Jeff Johnson

[permalink] [raw]
Subject: Re: [PATCH] cxl: add missing MODULE_DESCRIPTION() macros

On 6/7/2024 1:30 AM, Jonathan Cameron wrote:
> On Thu, 6 Jun 2024 16:42:44 -0700
> Dan Williams <[email protected]> wrote:
>
>> Jeff Johnson wrote:
>> [..]
>>>>> This I just made up from the others since config CXL_PORT doesn't have a menu
>>>>> description or help text and the .c file begins with:
>>>>> * DOC: cxl port
>>>>
>>>> "CXL: Port Support"
>>>>
>>>> Not that informative, but I can't immediately think of better text.
>>
>> How about "CXL: Port enumeration and services"
>>
>
> LGTM

Thanks, will update in v2