Re: [PATCH] Input: synaptics-rmi4 - make F03 a tristate symbol

On Jan 11 2017 or thereabouts, Arnd Bergmann wrote:

On Tuesday, January 10, 2017 4:39:43 PM CET Andrew Duggan wrote:

quoted

On 01/10/2017 04:16 AM, Arnd Bergmann wrote:

quoted

If CONFIG_INPUT=m, we get a build error for the rmi4-f03 driver,
added in linux-4.10:

drivers/input/built-in.o: In function `rmi_f03_attention':
rmi_f03.c:(.text+0xcfe0): undefined reference to `serio_interrupt'
rmi_f03.c:(.text+0xd055): undefined reference to `serio_interrupt'
drivers/input/built-in.o: In function `rmi_f03_remove':
rmi_f03.c:(.text+0xd115): undefined reference to `serio_unregister_port'
drivers/input/built-in.o: In function `rmi_f03_probe':
rmi_f03.c:(.text+0xd209): undefined reference to `__serio_register_port'

If we make the driver itself a 'tristate' instead of 'bool' symbol,
Kconfig ensures that it can only be a loadable module in this case,
which avoids the problem.

Unfortunately, the RMI4 driver does not support building the function 
drivers as modules. If F03 is built as a module it will not be loaded by 
the core. If we want f03 to be part of a module then rmi_core needs to 
be built as a module. We should remove the module macros currently in 
rmi_f03.c.

I was able to get a similar build error by setting CONFIG_RMI_CORE=y and 
CONFIG_SERIO=m. Was CONFIG_RMI_CORE=y set when you encountered this 
error? If so I think we should figure out a way to have Kconfig set 
CONFIG_RMI_CORE=m if serio is built as a module.

Ok, I see what you mean now in 

static struct rmi_function_handler *fn_handlers[] = {
        &rmi_f01_handler,
#ifdef CONFIG_RMI4_F03
        &rmi_f03_handler,
#endif
#ifdef CONFIG_RMI4_F11
        &rmi_f11_handler,
#endif
...
};

I think we can actually make this more modular and more like other
drivers work:

If each of the sub-drivers gets changed to call
rmi_register_function_handler() on its own handler structure,
having some drivers as modules would just work.

It looks like the rmi_bus.c file was written to do it that way,
but for some reason the references to those drivers are all
in the same file.

Yep, it was initially written that way, and IIRC there was some issues
depending on how the drivers were compiled. For example, if rmi4_core is
Y and some functions are m, you can't load the device initially, so you
send a -EPROBE_DEFER, but how can you be sure that the function will
ever be loaded?

Given that we need to have all the functions loaded during probe, we
decided to switch to a monolithic rmi4_core driver that has everything
it needs inside.

I think having the dependency on SERIO=m implying RMI4_CORE=m should be
doable and is probably the best solution with the current code.

Cheers,
Benjamin