Messages in this thread | | | Date | Sat, 13 May 2023 16:08:19 -0700 | Subject | Re: [RFC PATCH] powerpc: udbg: export udbg_putc | From | Randy Dunlap <> |
| |
On 5/13/23 16:05, Pali Rohár wrote: > On Saturday 13 May 2023 15:53:19 Randy Dunlap wrote: >> Hi Pali, >> >> On 5/13/23 02:13, Pali Rohár wrote: >>> On Friday 12 May 2023 22:58:04 Randy Dunlap wrote: >>>> In a randconfig with CONFIG_SERIAL_CPM=m and >>>> CONFIG_PPC_EARLY_DEBUG_CPM=y, there is a build error: >>>> ERROR: modpost: "udbg_putc" [drivers/tty/serial/cpm_uart/cpm_uart.ko] undefined! >>>> >>>> The build can be fixed by exporting "udbg_putc" in udbg.c. >>>> >>>> OTOH, maybe something like this is more appropriate in >>>> arch/powerpc/Kconfig.debug? >>>> >>>> config PPC_EARLY_DEBUG_CPM >>>> bool "Early serial debugging for Freescale CPM-based serial ports" >>>> - depends on SERIAL_CPM >>>> + depends on SERIAL_CPM=y >>>> >>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org> >>>> Cc: Michael Ellerman <mpe@ellerman.id.au> >>>> Cc: Nicholas Piggin <npiggin@gmail.com> >>>> Cc: Christophe Leroy <christophe.leroy@csgroup.eu> >>>> Cc: linuxppc-dev@lists.ozlabs.org >>>> Cc: Arnd Bergmann <arnd@arndb.de> >>>> Cc: "Pali Rohár" <pali@kernel.org> >>>> --- >>>> arch/powerpc/kernel/udbg.c | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff -- a/arch/powerpc/kernel/udbg.c b/arch/powerpc/kernel/udbg.c >>>> --- a/arch/powerpc/kernel/udbg.c >>>> +++ b/arch/powerpc/kernel/udbg.c >>>> @@ -14,6 +14,7 @@ >>>> #include <asm/udbg.h> >>>> >>>> void (*udbg_putc)(char c); >>>> +EXPORT_SYMBOL(udbg_putc); >>>> void (*udbg_flush)(void); >>>> int (*udbg_getc)(void); >>>> int (*udbg_getc_poll)(void); >>> >>> Hello! I do not think that it is a good idea to export udbg functions >>> for kernel modules. I have quickly looked at the cpm_uart driver file >> >> Sure, that was just a conversation starter. >> >>> drivers/tty/serial/cpm_uart/cpm_uart_core.c and it looks like that once >>> udbg from it is registered then there is no code for unregistering it. >>> So I have feeling that compiling cpm_uart driver as module should not be >>> allowed when CONFIG_SERIAL_CPM_CONSOLE is enabled, and early debug >>> should depend on CONFIG_CONSOLE_POLL || CONFIG_SERIAL_CPM_CONSOLE. >> >> The other change above also fixes the build error: (Option 2) >> >>>> config PPC_EARLY_DEBUG_CPM >>>> bool "Early serial debugging for Freescale CPM-based serial ports" >>>> - depends on SERIAL_CPM >>>> + depends on SERIAL_CPM=y >> >> >> Also, making SERIAL_CPM (cpm_uart driver) be restricted to not allow it >> as a module when SERIAL_CPM_CONSOLE is enabled [how does one express that >> in Kconfig language?] will cause a circular dependency since SERIAL_CPM_CONSOLE >> depends on SERIAL_CPM=y. It looks like you are suggesting reversing the >> dependencies. > > I see that SERIAL_CPM_CONSOLE has already "depends on SERIAL_CPM=y". > Am I right that this already disallow compiling cpm_uart driver as > module when SERIAL_CPM_CONSOLE is enabled?
No, it just prevents SERIAL_CPM_CONSOLE from being set when SERIAL_CPM=m.
>>> Any other opinion? >> >> Yes, please. Otherwise I prefer my option 2. >> >> -- >> ~Randy > > Option 2 looks like a better solution.
Thanks. -- ~Randy
| |