On 6/6/2024 7:15 AM, Jonathan Cameron wrote: > On Tue, 4 Jun 2024 13:21:52 -0700 > Jeff Johnson <quic_jjohnson@xxxxxxxxxxx> wrote: > >> On 6/4/2024 9:04 AM, Jonathan Cameron wrote: >>> On Mon, 3 Jun 2024 21:48:53 -0700 >>> Jeff Johnson <quic_jjohnson@xxxxxxxxxxx> 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 <quic_jjohnson@xxxxxxxxxxx> >>> >>> 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