Re: [PATCH 3/3] Bluetooth: Add MGMT_OP_SET_LE_SUPPORT command

Hi Andre,

quoted

quoted

quoted

any reason why we want this separated and being under host stack control
in the first place. Should we not always enable LE if we can support it?
What are the reason to run an LE capable BlueZ on an LE capable dongle
and then not enable LE?

I don't know the reason why, but today LE enabling is separated and is
under host control. There is an option in main.conf (EnableLE) to
enable/disable LE.

AFAIK, the reason we want to disable LE is qualification tests only. Also,
since LE support is under development we may not wanna have LE enabled
by default (maybe this is another reason).

for that I would have used a enable_le kernel module option since it
should be all triggered by the kernel anyway.

quoted

quoted

Maybe we should just have a generic kernel mgmt features enabling
command. One thing that I do not wanna do is to just blindly make a copy
of HCI commands for mgmt interface. The controller abstraction for the
mgmt interface is suppose to have a feature list so we can check what
the kernel supports and what not.

We had a little discussion here and we came up with this idea of having
new parameter to the module (e. g. enable_le) to enable/disable LE. If
the module is loaded with enable_le=y we would enable LE host support.
Once LE support is stable enough, we have enable_le=y by default.
This approach we don't need a mgmt command.

See above ;)

I am open for discussion on how we might solve this in a bit more
generic way on let this control by the user. Mainly for qualification
testing and UPF testing. However we need a clear story for LE and SSP
since both are host stack driven features.

For LE story, the enable_le module param should be enough. IMHO, LE
support shouldn't be controlled by the user. If hardware, kernel and BlueZ
support LE then LE should be enabled.

If you have LE supported hardware, kernel and BlueZ but for some reason
(e.g. qualification tests) you want to disable LE, then you load the module
with enable_le=n parameter.

For now, while LE support is under development, enable_le default value
would be false. Once we have LE support stable enough, we change its
default value to true.

sounds good to me. I am always for the minimal approach. Since adding
something later is easy. Removing/deprecating is hard. So if we feel
like a mgmt command is needed, we can add it any time.

Regards

Marcel