Bluetooth: btrsi: rework dependencies

The linkage between the bluetooth driver and the wireless
driver is not defined properly, leading to build problems
such as:

warning: (BT_HCIRSI) selects RSI_COEX which has unmet direct dependencies (NETDEVICES && WLAN && WLAN_VENDOR_RSI && BT_HCIRSI && RSI_91X)
drivers/net/wireless/rsi/rsi_91x_main.o: In function `rsi_read_pkt':
(.text+0x205): undefined reference to `rsi_bt_ops'

To actually make it work, we need the following steps:

- remove the bogus 'select RSI_COEX', this is already covered
  by the default
- change RSI_COEX to a 'bool' symbol so that the #ifdefs work
  all the time
- ensure that BT_HCIRSI is built-in whenever RSI_91X is built-in
- prevent BT_HCIRSI from being enabled when CONFIG_BT=m and
  RSI_91X=y, as this cannot work

Fixes: 38aa4da50483 ("Bluetooth: btrsi: add new rsi bluetooth driver")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 drivers/bluetooth/Kconfig        | 9 ++++++++-
 drivers/bluetooth/Makefile       | 2 +-
 drivers/net/wireless/rsi/Kconfig | 6 +-----
 3 files changed, 10 insertions(+), 7 deletions(-)

-- 
2.9.0

Hi Arnd,

> The linkage between the bluetooth driver and the wireless

> driver is not defined properly, leading to build problems

> such as:

> 

> warning: (BT_HCIRSI) selects RSI_COEX which has unmet direct dependencies (NETDEVICES && WLAN && WLAN_VENDOR_RSI && BT_HCIRSI && RSI_91X)

> drivers/net/wireless/rsi/rsi_91x_main.o: In function `rsi_read_pkt':

> (.text+0x205): undefined reference to `rsi_bt_ops'

> 

> To actually make it work, we need the following steps:

> 

> - remove the bogus 'select RSI_COEX', this is already covered

>  by the default

> - change RSI_COEX to a 'bool' symbol so that the #ifdefs work

>  all the time

> - ensure that BT_HCIRSI is built-in whenever RSI_91X is built-in

> - prevent BT_HCIRSI from being enabled when CONFIG_BT=m and

>  RSI_91X=y, as this cannot work

> 

> Fixes: 38aa4da50483 ("Bluetooth: btrsi: add new rsi bluetooth driver")

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> ---

> drivers/bluetooth/Kconfig        | 9 ++++++++-

> drivers/bluetooth/Makefile       | 2 +-

> drivers/net/wireless/rsi/Kconfig | 6 +-----

> 3 files changed, 10 insertions(+), 7 deletions(-)

> 

> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig

> index d8bbd661dbdb..6b3e8bf69d07 100644

> --- a/drivers/bluetooth/Kconfig

> +++ b/drivers/bluetooth/Kconfig

> @@ -394,8 +394,9 @@ config BT_QCOMSMD

> 

> config BT_HCIRSI

> 	tristate "Redpine HCI support"

> +	depends on RSI_91X

> +	depends on !(BT=m && RSI_91X=y)

> 	default n

> -	select RSI_COEX

> 	help

> 	  Redpine BT driver.

> 	  This driver handles BT traffic from upper layers and pass

> @@ -404,4 +405,10 @@ config BT_HCIRSI

> 	  Say Y here to compile support for HCI over Redpine into the

> 	  kernel or say M to compile as a module.

> 

> +config BT_HCIRSI_MODULE

> +	tristate

> +	# ensure that the BT_HCIRSI driver is visible to the core

> +	default y if BT_HCIRSI=m && RSI_91X=y

> +	default BT_HCIRSI

> +

> endmenu

> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile

> index 03cfc1b20c4a..9e8d22712ff3 100644

> --- a/drivers/bluetooth/Makefile

> +++ b/drivers/bluetooth/Makefile

> @@ -28,7 +28,7 @@ obj-$(CONFIG_BT_QCA)		+= btqca.o

> 

> obj-$(CONFIG_BT_HCIUART_NOKIA)	+= hci_nokia.o

> 

> -obj-$(CONFIG_BT_HCIRSI)		+= btrsi.o

> +obj-$(CONFIG_BT_HCIRSI_MODULE)	+= btrsi.o


do we need this new option? I have avoided these kind of complex things multi config entries. Can we not just select the RSI_91X?

Regards

Marcel

On Thu, Mar 15, 2018 at 4:50 PM, Marcel Holtmann <marcel@holtmann.org> wrote:

>> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile

>> index 03cfc1b20c4a..9e8d22712ff3 100644

>> --- a/drivers/bluetooth/Makefile

>> +++ b/drivers/bluetooth/Makefile

>> @@ -28,7 +28,7 @@ obj-$(CONFIG_BT_QCA)                += btqca.o

>>

>> obj-$(CONFIG_BT_HCIUART_NOKIA)        += hci_nokia.o

>>

>> -obj-$(CONFIG_BT_HCIRSI)              += btrsi.o

>> +obj-$(CONFIG_BT_HCIRSI_MODULE)       += btrsi.o

>

> do we need this new option? I have avoided these kind of complex things multi config entries. Can we not just select the RSI_91X?

>


I couldn't come up with a simpler way to do this.
Selecting RSI_91X is not possible unless we make the BT
driver 'depend on WLAN_VENDOR_RSI && MAC80211',
which is even more backwards.

The problem here is that it's actually a reverse dependency:
the wlan driver calls into the bt driver.

What we could do is to make BT_HCIRSI a silent symbol
and have that selected by RSI_COEX, which can then
be user-visible. With that, the Kconfig structure would follow
what the code does.

       Arnd

Hi Arnd,

>>> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile

>>> index 03cfc1b20c4a..9e8d22712ff3 100644

>>> --- a/drivers/bluetooth/Makefile

>>> +++ b/drivers/bluetooth/Makefile

>>> @@ -28,7 +28,7 @@ obj-$(CONFIG_BT_QCA)                += btqca.o

>>> 

>>> obj-$(CONFIG_BT_HCIUART_NOKIA)        += hci_nokia.o

>>> 

>>> -obj-$(CONFIG_BT_HCIRSI)              += btrsi.o

>>> +obj-$(CONFIG_BT_HCIRSI_MODULE)       += btrsi.o

>> 

>> do we need this new option? I have avoided these kind of complex things multi config entries. Can we not just select the RSI_91X?

>> 

> 

> I couldn't come up with a simpler way to do this.

> Selecting RSI_91X is not possible unless we make the BT

> driver 'depend on WLAN_VENDOR_RSI && MAC80211',

> which is even more backwards.

> 

> The problem here is that it's actually a reverse dependency:

> the wlan driver calls into the bt driver.

> 

> What we could do is to make BT_HCIRSI a silent symbol

> and have that selected by RSI_COEX, which can then

> be user-visible. With that, the Kconfig structure would follow

> what the code does.


that sounds a bit better to me. If the RSI driver maintainers don’t like that, then they should re-architect the code to make this more dynamic and flexible.

Regards

Marcel