[PATCH] Bluetooth: AMP: Register AMP only if High Speed enabled

[PATCH] Bluetooth: AMP: Register AMP only if High Speed enabled

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Make sure that AMP controller is registered only if HS enables.
Return EINVAL if High Speed is not enabled and dev_type is HCI_AMP.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 net/bluetooth/hci_core.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index bd26cb5..746af2d 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1708,7 +1708,10 @@ int hci_register_dev(struct hci_dev *hdev)
 		id = ida_simple_get(&hci_index_ida, 0, 0, GFP_KERNEL);
 		break;
 	case HCI_AMP:
-		id = ida_simple_get(&hci_index_ida, 1, 0, GFP_KERNEL);
+		if (enable_hs)
+			id = ida_simple_get(&hci_index_ida, 1, 0, GFP_KERNEL);
+		else
+			return -EINVAL;
 		break;
 	default:
 		return -EINVAL;
-- 
1.7.9.5

Re: [PATCH] Bluetooth: AMP: Register AMP only if High Speed enabled

[Marcel Holtmann]

Hi Andrei,

> Make sure that AMP controller is registered only if HS enables.
> Return EINVAL if High Speed is not enabled and dev_type is HCI_AMP.

I don't think this is a good idea. I like to keep the adapter registers,
but maybe have a hciconfig hciX up fail. Not the registration itself.

If you would fail the registration itself, then you have the problem
that you need to unplug and replug the hardware once you changed
enable_hs. And that might not always be possible.

Regards

Marcel

Re: [PATCH] Bluetooth: AMP: Register AMP only if High Speed enabled

[Andrei Emeltchenko]

Hi Marcel,

On Thu, Oct 04, 2012 at 11:52:11AM +0200, Marcel Holtmann wrote:
> Hi Andrei,
> 
> > Make sure that AMP controller is registered only if HS enables.
> > Return EINVAL if High Speed is not enabled and dev_type is HCI_AMP.
> 
> I don't think this is a good idea. I like to keep the adapter registers,
> but maybe have a hciconfig hciX up fail. Not the registration itself.

So currently AMP registers itself as RAW device, maybe leave it as is.

Best regards 
Andrei Emeltchenko 

> 
> If you would fail the registration itself, then you have the problem
> that you need to unplug and replug the hardware once you changed
> enable_hs. And that might not always be possible.
> 
> Regards
> 
> Marcel
> 
> 

Re: [PATCH] Bluetooth: AMP: Register AMP only if High Speed enabled

[Marcel Holtmann]

Hi Andrei,

> > > Make sure that AMP controller is registered only if HS enables.
> > > Return EINVAL if High Speed is not enabled and dev_type is HCI_AMP.
> > 
> > I don't think this is a good idea. I like to keep the adapter registers,
> > but maybe have a hciconfig hciX up fail. Not the registration itself.
> 
> So currently AMP registers itself as RAW device, maybe leave it as is.

we can do that, but that will also never give us enough testing
exposure. An adapter registered a RAW device, can not undo the RAW
device from userspace, you need to recompile the kernel.

So as I said, I rather have hciconfig hciX up fail in the case enable_hs
is set to false.

I am even fine with allowing to bring up AMP controllers. Just when it
comes to A2MP, you can not announce them to the other side unless
enable_hs is set to true.

You can not even list A2MP in the fixed channel list if enanble_hs is
false and L2CAP and A2MP procedures should just fail or return an error.

Regards

Marcel