2023-12-04 12:04:05

by kernel test robot

[permalink] [raw]
Subject: pm.c:undefined reference to `i8042_command'

tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 33cc938e65a98f1d29d0a18403dbbee050dcad9a
commit: 7b4537199a4a8480b8c3ba37a2d44765ce76cd9b kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS
date: 1 year, 6 months ago
config: mips-randconfig-r001-20211012 (https://download.01.org/0day-ci/archive/20231204/[email protected]/config)
compiler: mips64el-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231204/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

mips64el-linux-ld: arch/mips/loongson2ef/lemote-2f/pm.o: in function `setup_wakeup_events':
>> pm.c:(.text+0x118): undefined reference to `i8042_command'
>> mips64el-linux-ld: pm.c:(.text+0x154): undefined reference to `i8042_command'

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


2023-12-06 05:24:48

by Randy Dunlap

[permalink] [raw]
Subject: Re: pm.c:undefined reference to `i8042_command'

Hi,


On 12/4/23 04:02, kernel test robot wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 33cc938e65a98f1d29d0a18403dbbee050dcad9a
> commit: 7b4537199a4a8480b8c3ba37a2d44765ce76cd9b kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS
> date: 1 year, 6 months ago
> config: mips-randconfig-r001-20211012 (https://download.01.org/0day-ci/archive/20231204/[email protected]/config)
> compiler: mips64el-linux-gcc (GCC) 13.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231204/[email protected]/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <[email protected]>
> | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
>
> All errors (new ones prefixed by >>):
>
> mips64el-linux-ld: arch/mips/loongson2ef/lemote-2f/pm.o: in function `setup_wakeup_events':
>>> pm.c:(.text+0x118): undefined reference to `i8042_command'
>>> mips64el-linux-ld: pm.c:(.text+0x154): undefined reference to `i8042_command'


How do we feel about this?
I suppose that an ARCH or mach or board should know what it requires.


---
arch/mips/loongson2ef/Kconfig | 3 +++
1 file changed, 3 insertions(+)

diff -- a/arch/mips/loongson2ef/Kconfig b/arch/mips/loongson2ef/Kconfig
--- a/arch/mips/loongson2ef/Kconfig
+++ b/arch/mips/loongson2ef/Kconfig
@@ -40,6 +40,9 @@ config LEMOTE_MACH2F
select ARCH_HAS_PHYS_TO_DMA
select ARCH_MIGHT_HAVE_PC_PARPORT
select ARCH_MIGHT_HAVE_PC_SERIO
+ select INPUT
+ select SERIO
+ select SERIO_I8042
select BOARD_SCACHE
select BOOT_ELF32
select CEVT_R4K if ! MIPS_EXTERNAL_TIMER


--
~Randy

2023-12-06 08:31:22

by Arnd Bergmann

[permalink] [raw]
Subject: Re: pm.c:undefined reference to `i8042_command'

On Wed, Dec 6, 2023, at 06:24, Randy Dunlap wrote:
>> All errors (new ones prefixed by >>):
>>
>> mips64el-linux-ld: arch/mips/loongson2ef/lemote-2f/pm.o: in function `setup_wakeup_events':
>>>> pm.c:(.text+0x118): undefined reference to `i8042_command'
>>>> mips64el-linux-ld: pm.c:(.text+0x154): undefined reference to `i8042_command'
>
>
> How do we feel about this?
> I suppose that an ARCH or mach or board should know what it requires.
>
>
> ---
> arch/mips/loongson2ef/Kconfig | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff -- a/arch/mips/loongson2ef/Kconfig b/arch/mips/loongson2ef/Kconfig
> --- a/arch/mips/loongson2ef/Kconfig
> +++ b/arch/mips/loongson2ef/Kconfig
> @@ -40,6 +40,9 @@ config LEMOTE_MACH2F
> select ARCH_HAS_PHYS_TO_DMA
> select ARCH_MIGHT_HAVE_PC_PARPORT
> select ARCH_MIGHT_HAVE_PC_SERIO
> + select INPUT
> + select SERIO
> + select SERIO_I8042
> select BOARD_SCACHE
> select BOOT_ELF32

I think it's bad style to force-select an optional subsystem.
How about making the entire file optional? It seems that there
are already __weak functions in its place.

--- a/arch/mips/loongson2ef/lemote-2f/Makefile
+++ b/arch/mips/loongson2ef/lemote-2f/Makefile
@@ -8,5 +8,6 @@ obj-y += clock.o machtype.o irq.o reset.o dma.o ec_kb3310b.o
#
# Suspend Support
#
-
+ifdef CONFIG_SERIO_I8042
obj-$(CONFIG_SUSPEND) += pm.o
+endif

Arnd

2023-12-06 17:26:35

by Randy Dunlap

[permalink] [raw]
Subject: Re: pm.c:undefined reference to `i8042_command'



On 12/5/23 22:52, Arnd Bergmann wrote:
> On Wed, Dec 6, 2023, at 06:24, Randy Dunlap wrote:
>>> All errors (new ones prefixed by >>):
>>>
>>> mips64el-linux-ld: arch/mips/loongson2ef/lemote-2f/pm.o: in function `setup_wakeup_events':
>>>>> pm.c:(.text+0x118): undefined reference to `i8042_command'
>>>>> mips64el-linux-ld: pm.c:(.text+0x154): undefined reference to `i8042_command'
>>
>>
>> How do we feel about this?
>> I suppose that an ARCH or mach or board should know what it requires.
>>
>>
>> ---
>> arch/mips/loongson2ef/Kconfig | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff -- a/arch/mips/loongson2ef/Kconfig b/arch/mips/loongson2ef/Kconfig
>> --- a/arch/mips/loongson2ef/Kconfig
>> +++ b/arch/mips/loongson2ef/Kconfig
>> @@ -40,6 +40,9 @@ config LEMOTE_MACH2F
>> select ARCH_HAS_PHYS_TO_DMA
>> select ARCH_MIGHT_HAVE_PC_PARPORT
>> select ARCH_MIGHT_HAVE_PC_SERIO
>> + select INPUT
>> + select SERIO
>> + select SERIO_I8042
>> select BOARD_SCACHE
>> select BOOT_ELF32
>
> I think it's bad style to force-select an optional subsystem.
> How about making the entire file optional? It seems that there
> are already __weak functions in its place.

Yes, I agree in general.

Hopefully the maintainer will opine your suggestion.
Jiaxun?

Thanks.

>
> --- a/arch/mips/loongson2ef/lemote-2f/Makefile
> +++ b/arch/mips/loongson2ef/lemote-2f/Makefile
> @@ -8,5 +8,6 @@ obj-y += clock.o machtype.o irq.o reset.o dma.o ec_kb3310b.o
> #
> # Suspend Support
> #
> -
> +ifdef CONFIG_SERIO_I8042
> obj-$(CONFIG_SUSPEND) += pm.o
> +endif
>
> Arnd

--
~Randy

2023-12-07 01:27:04

by Jiaxun Yang

[permalink] [raw]
Subject: Re: pm.c:undefined reference to `i8042_command'



在2023年12月6日十二月 下午5:26,Randy Dunlap写道:
> On 12/5/23 22:52, Arnd Bergmann wrote:
>> On Wed, Dec 6, 2023, at 06:24, Randy Dunlap wrote:
>>>> All errors (new ones prefixed by >>):
>>>>
>>>> mips64el-linux-ld: arch/mips/loongson2ef/lemote-2f/pm.o: in function `setup_wakeup_events':
>>>>>> pm.c:(.text+0x118): undefined reference to `i8042_command'
>>>>>> mips64el-linux-ld: pm.c:(.text+0x154): undefined reference to `i8042_command'
>>>
>>>
>>> How do we feel about this?
>>> I suppose that an ARCH or mach or board should know what it requires.
>>>
>>>
>>> ---
>>> arch/mips/loongson2ef/Kconfig | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff -- a/arch/mips/loongson2ef/Kconfig b/arch/mips/loongson2ef/Kconfig
>>> --- a/arch/mips/loongson2ef/Kconfig
>>> +++ b/arch/mips/loongson2ef/Kconfig
>>> @@ -40,6 +40,9 @@ config LEMOTE_MACH2F
>>> select ARCH_HAS_PHYS_TO_DMA
>>> select ARCH_MIGHT_HAVE_PC_PARPORT
>>> select ARCH_MIGHT_HAVE_PC_SERIO
>>> + select INPUT
>>> + select SERIO
>>> + select SERIO_I8042
>>> select BOARD_SCACHE
>>> select BOOT_ELF32
>>
>> I think it's bad style to force-select an optional subsystem.
>> How about making the entire file optional? It seems that there
>> are already __weak functions in its place.
>
> Yes, I agree in general.
>
> Hopefully the maintainer will opine your suggestion.
> Jiaxun?

LGTM, PM is not an essential function here.
Arnd, do you mind making it a patch?

Thanks
- Jiaxun

>
> Thanks.
>
>>
>> --- a/arch/mips/loongson2ef/lemote-2f/Makefile
>> +++ b/arch/mips/loongson2ef/lemote-2f/Makefile
>> @@ -8,5 +8,6 @@ obj-y += clock.o machtype.o irq.o reset.o dma.o ec_kb3310b.o
>> #
>> # Suspend Support
>> #
>> -
>> +ifdef CONFIG_SERIO_I8042
>> obj-$(CONFIG_SUSPEND) += pm.o
>> +endif
>>
>> Arnd
>
> --
> ~Randy

--
- Jiaxun

2023-12-14 23:57:54

by Randy Dunlap

[permalink] [raw]
Subject: Re: pm.c:undefined reference to `i8042_command'



On 12/14/23 11:54, Arnd Bergmann wrote:
> On Thu, Dec 7, 2023, at 01:21, Jiaxun Yang wrote:
>> 在2023年12月6日十二月 下午5:26,Randy Dunlap写道:
>>> On 12/5/23 22:52, Arnd Bergmann wrote:
>>>> On Wed, Dec 6, 2023, at 06:24, Randy Dunlap wrote:
>
>>>> I think it's bad style to force-select an optional subsystem.
>>>> How about making the entire file optional? It seems that there
>>>> are already __weak functions in its place.
>>>
>>> Yes, I agree in general.
>>>
>>> Hopefully the maintainer will opine your suggestion.
>>> Jiaxun?
>>
>> LGTM, PM is not an essential function here.
>> Arnd, do you mind making it a patch?
>>
>
> I just tried it out and my version doesn't actually work since
> we still end up building the file with SERIO_I82042=m.
>
> We could still make it work by checking for serio being
> built-in here, but in the end I think that's worse than
> Randy's patch, so let's just go with his original version.
>
> Arnd

I think that we should look for yet another alternative then.
I don't believe that adding "select INPUT" is a good idea.
It would be the only "select INPUT" in the entire kernel tree.


thanks.
--
#Randy
https://people.kernel.org/tglx/notes-about-netiquette
https://subspace.kernel.org/etiquette.html

2023-12-15 06:54:08

by Arnd Bergmann

[permalink] [raw]
Subject: Re: pm.c:undefined reference to `i8042_command'

On Thu, Dec 7, 2023, at 01:21, Jiaxun Yang wrote:
> 在2023年12月6日十二月 下午5:26,Randy Dunlap写道:
>> On 12/5/23 22:52, Arnd Bergmann wrote:
>>> On Wed, Dec 6, 2023, at 06:24, Randy Dunlap wrote:

>>> I think it's bad style to force-select an optional subsystem.
>>> How about making the entire file optional? It seems that there
>>> are already __weak functions in its place.
>>
>> Yes, I agree in general.
>>
>> Hopefully the maintainer will opine your suggestion.
>> Jiaxun?
>
> LGTM, PM is not an essential function here.
> Arnd, do you mind making it a patch?
>

I just tried it out and my version doesn't actually work since
we still end up building the file with SERIO_I82042=m.

We could still make it work by checking for serio being
built-in here, but in the end I think that's worse than
Randy's patch, so let's just go with his original version.

Arnd