Re: [PATCH] serdev: Update drivers/tty/serdev/Kconfig for ACPI support

[Johan Hovold <johan@kernel.org>]

On Sat, Oct 07, 2017 at 09:57:40PM +0200, Marcel Holtmann wrote:
> Hi Johan,
> 
> >> The current Kconfig for serdev is not compatible when adding ACPI support as it does not work when built as a module as it requires config SERIAL_DEV_CTRL_TTYPORT to be set. This patch makes serdev compiled into the kernel if selected so that config SERIAL_DEV_CTRL_TTYPORT can be correctly set if requiring ACPI support.
> >> ---
> >> drivers/tty/serdev/Kconfig | 5 +++--
> >> 1 file changed, 3 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/drivers/tty/serdev/Kconfig b/drivers/tty/serdev/Kconfig
> >> index cdc6b820cf93..a9fb09a9c105 100644
> >> --- a/drivers/tty/serdev/Kconfig
> >> +++ b/drivers/tty/serdev/Kconfig
> >> @@ -2,7 +2,8 @@
> >> # Serial bus device driver configuration
> >> #
> >> menuconfig SERIAL_DEV_BUS
> >> -	tristate "Serial device bus"
> >> +	bool "Serial device bus"
> >> +	default y
> > 
> > I understand why you want this (to prevent hci_bcm from breaking), but we
> > should generally not have new entries default to y.
> > 
> >> 	help
> >> 	  Core support for devices connected via a serial port.
> >> 
> >> @@ -11,6 +12,6 @@ if SERIAL_DEV_BUS
> >> config SERIAL_DEV_CTRL_TTYPORT
> >> 	bool "Serial device TTY port controller"
> >> 	depends on TTY
> >> -	depends on SERIAL_DEV_BUS != m
> >> +	default y
> > 
> > Same here.
> > 
> > It may be better to have BT_HCIUART_BCM depend on (or select?)
> > SERIAL_DEV_CTRL_TTYPORT instead.
> 
> if we move SERIAL_DEV_BUS to bool, then I would just have it be
> selected by BT_HCIUART_BCM. Frankly the SERIAL_DEV_BUS option is
> pretty hard to find in the kernel config. And if we depend on TTY, but
> then select SERIAL_DEV_BUS, I think that is a good compromise.

The Bluetooth UART drivers already depend on on SERIAL_DEV_BUS (through
BT_HCIUART_SERDEV). And the problem with BT_HCIUART_BCM is that the
current ACPI devices really need SERIAL_DEV_CTRL_TTYPORT (and not just
serdev core).

And we should probably try to avoid selecting options if we can (to
avoid ending up with unmet dependencies).

Johan