[PATCHv3 0/3] Trivial fixes for emulating AMP HCI

[PATCHv3 0/3] Trivial fixes for emulating AMP HCI

[Emeltchenko Andrei]

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

Changes:
	v3 Changed parameter description
        v2 Taken Marcel's comments about parameter name & permissions.

Those trivial patches help to emulate AMP HCI controller
currently used for A2MP protocol verification.

Andrei Emeltchenko (3):
  Bluetooth: Allow to set AMP type for virtual HCI
  Bluetooth: Move scope of kernel parameter enable_hs
  Bluetooth: Do not set HCI_RAW when HS enabled

 drivers/bluetooth/hci_vhci.c  |    8 ++++++++
 include/net/bluetooth/hci.h   |    2 ++
 include/net/bluetooth/l2cap.h |    1 -
 net/bluetooth/hci_core.c      |    5 +++--
 4 files changed, 13 insertions(+), 3 deletions(-)

-- 
1.7.4.1

[PATCHv3 1/3] Bluetooth: Allow to set AMP type for virtual HCI

[Emeltchenko Andrei]

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

Type can be changed during re-opening device /dev/vhci.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 drivers/bluetooth/hci_vhci.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/bluetooth/hci_vhci.c b/drivers/bluetooth/hci_vhci.c
index 2e302a1..2ed6ab1 100644
--- a/drivers/bluetooth/hci_vhci.c
+++ b/drivers/bluetooth/hci_vhci.c
@@ -41,6 +41,8 @@
 
 #define VERSION "1.3"
 
+static bool amp;
+
 struct vhci_data {
 	struct hci_dev *hdev;
 
@@ -239,6 +241,9 @@ static int vhci_open(struct inode *inode, struct file *file)
 	hdev->bus = HCI_VIRTUAL;
 	hdev->driver_data = data;
 
+	if (amp)
+		hdev->dev_type = HCI_AMP;
+
 	hdev->open     = vhci_open_dev;
 	hdev->close    = vhci_close_dev;
 	hdev->flush    = vhci_flush;
@@ -303,6 +308,9 @@ static void __exit vhci_exit(void)
 module_init(vhci_init);
 module_exit(vhci_exit);
 
+module_param(amp, bool, 0644);
+MODULE_PARM_DESC(amp, "Create AMP controller device");
+
 MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>");
 MODULE_DESCRIPTION("Bluetooth virtual HCI driver ver " VERSION);
 MODULE_VERSION(VERSION);
-- 
1.7.4.1

Re: [PATCHv3 1/3] Bluetooth: Allow to set AMP type for virtual HCI

[Emeltchenko Andrei]

Hi,

On Mon, Nov 14, 2011 at 12:42:48PM +0200, Emeltchenko Andrei wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> Type can be changed during re-opening device /dev/vhci.
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> Acked-by: Marcel Holtmann <marcel@holtmann.org>
> ---
>  drivers/bluetooth/hci_vhci.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/bluetooth/hci_vhci.c b/drivers/bluetooth/hci_vhci.c
> index 2e302a1..2ed6ab1 100644
> --- a/drivers/bluetooth/hci_vhci.c
> +++ b/drivers/bluetooth/hci_vhci.c
> @@ -41,6 +41,8 @@
>  
>  #define VERSION "1.3"
>  
> +static bool amp;
> +
>  struct vhci_data {
>  	struct hci_dev *hdev;
>  
> @@ -239,6 +241,9 @@ static int vhci_open(struct inode *inode, struct file *file)
>  	hdev->bus = HCI_VIRTUAL;
>  	hdev->driver_data = data;
>  
> +	if (amp)
> +		hdev->dev_type = HCI_AMP;
> +

I am thinking about changing lines above to:

-       if (amp)
+       if (amp) {
                hdev->dev_type = HCI_AMP;
+               hdev->amp_type = AMP_CTRL_BLUETOOTH_ONLY;
+       }

to indicate that that "the AMP Controller is only used by the Bluetooth
technology and will not be shared with other non-Bluetooth technologies".

Best regards 
Andrei Emeltchenko 

>  	hdev->open     = vhci_open_dev;
>  	hdev->close    = vhci_close_dev;
>  	hdev->flush    = vhci_flush;
> @@ -303,6 +308,9 @@ static void __exit vhci_exit(void)
>  module_init(vhci_init);
>  module_exit(vhci_exit);
>  
> +module_param(amp, bool, 0644);
> +MODULE_PARM_DESC(amp, "Create AMP controller device");
> +
>  MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>");
>  MODULE_DESCRIPTION("Bluetooth virtual HCI driver ver " VERSION);
>  MODULE_VERSION(VERSION);
> -- 
> 1.7.4.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 1/3] Bluetooth: Allow to set AMP type for virtual HCI

[Emeltchenko Andrei]

Hi,

On Tue, Nov 15, 2011 at 02:09:26PM +0200, Emeltchenko Andrei wrote:
> > @@ -239,6 +241,9 @@ static int vhci_open(struct inode *inode, struct file *file)
> >  	hdev->bus = HCI_VIRTUAL;
> >  	hdev->driver_data = data;
> >  
> > +	if (amp)
> > +		hdev->dev_type = HCI_AMP;
> > +
> 
> I am thinking about changing lines above to:
> 
> -       if (amp)
> +       if (amp) {
>                 hdev->dev_type = HCI_AMP;
> +               hdev->amp_type = AMP_CTRL_BLUETOOTH_ONLY;

actually it is amp_status, I have resent patches and removed ack from this
specific patch.

Best regards 
Andrei Emeltchenko  

Re: [PATCHv3 1/3] Bluetooth: Allow to set AMP type for virtual HCI

[Marcel Holtmann]

Hi Andrei,

> > Type can be changed during re-opening device /dev/vhci.
> > 
> > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> > Acked-by: Marcel Holtmann <marcel@holtmann.org>
> > ---
> >  drivers/bluetooth/hci_vhci.c |    8 ++++++++
> >  1 files changed, 8 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/bluetooth/hci_vhci.c b/drivers/bluetooth/hci_vhci.c
> > index 2e302a1..2ed6ab1 100644
> > --- a/drivers/bluetooth/hci_vhci.c
> > +++ b/drivers/bluetooth/hci_vhci.c
> > @@ -41,6 +41,8 @@
> >  
> >  #define VERSION "1.3"
> >  
> > +static bool amp;
> > +
> >  struct vhci_data {
> >  	struct hci_dev *hdev;
> >  
> > @@ -239,6 +241,9 @@ static int vhci_open(struct inode *inode, struct file *file)
> >  	hdev->bus = HCI_VIRTUAL;
> >  	hdev->driver_data = data;
> >  
> > +	if (amp)
> > +		hdev->dev_type = HCI_AMP;
> > +
> 
> I am thinking about changing lines above to:
> 
> -       if (amp)
> +       if (amp) {
>                 hdev->dev_type = HCI_AMP;
> +               hdev->amp_type = AMP_CTRL_BLUETOOTH_ONLY;
> +       }
> 
> to indicate that that "the AMP Controller is only used by the Bluetooth
> technology and will not be shared with other non-Bluetooth technologies".

this can not be static information at this point. It will change
depending on how the WiFi subsystem or WiFi drivers want it to be.

And lets cross the bridge when we get to it. This is too early to figure
out what to do here.

Regards

Marcel

Re: [PATCHv3 1/3] Bluetooth: Allow to set AMP type for virtual HCI

[Emeltchenko Andrei]

Hi Marcel,

On Wed, Nov 16, 2011 at 10:13:10AM +0900, Marcel Holtmann wrote:
> Hi Andrei,
> 
> > > Type can be changed during re-opening device /dev/vhci.
> > > 
> > > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> > > Acked-by: Marcel Holtmann <marcel@holtmann.org>
> > > ---
> > >  drivers/bluetooth/hci_vhci.c |    8 ++++++++
> > >  1 files changed, 8 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/drivers/bluetooth/hci_vhci.c b/drivers/bluetooth/hci_vhci.c
> > > index 2e302a1..2ed6ab1 100644
> > > --- a/drivers/bluetooth/hci_vhci.c
> > > +++ b/drivers/bluetooth/hci_vhci.c
> > > @@ -41,6 +41,8 @@
> > >  
> > >  #define VERSION "1.3"
> > >  
> > > +static bool amp;
> > > +
> > >  struct vhci_data {
> > >  	struct hci_dev *hdev;
> > >  
> > > @@ -239,6 +241,9 @@ static int vhci_open(struct inode *inode, struct file *file)
> > >  	hdev->bus = HCI_VIRTUAL;
> > >  	hdev->driver_data = data;
> > >  
> > > +	if (amp)
> > > +		hdev->dev_type = HCI_AMP;
> > > +
> > 
> > I am thinking about changing lines above to:
> > 
> > -       if (amp)
> > +       if (amp) {
> >                 hdev->dev_type = HCI_AMP;
> > +               hdev->amp_type = AMP_CTRL_BLUETOOTH_ONLY;
> > +       }
> > 
> > to indicate that that "the AMP Controller is only used by the Bluetooth
> > technology and will not be shared with other non-Bluetooth technologies".
> 
> this can not be static information at this point. It will change
> depending on how the WiFi subsystem or WiFi drivers want it to be.
> 
> And lets cross the bridge when we get to it. This is too early to figure
> out what to do here.

The code only emulates virtual HCI amp_type. The reason why I added it is
to reply for A2MP Discover Req. If we leave field as 0 it would mean that
that controller is powered down.

This value can be read with "read local amp info", I can implement this
command in hciemu as other option.

Best regards 
Andrei Emeltchenko 

Re: [PATCHv3 1/3] Bluetooth: Allow to set AMP type for virtual HCI

[Marcel Holtmann]

Hi Andrei,

> > > > Type can be changed during re-opening device /dev/vhci.
> > > > 
> > > > Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> > > > Acked-by: Marcel Holtmann <marcel@holtmann.org>
> > > > ---
> > > >  drivers/bluetooth/hci_vhci.c |    8 ++++++++
> > > >  1 files changed, 8 insertions(+), 0 deletions(-)
> > > > 
> > > > diff --git a/drivers/bluetooth/hci_vhci.c b/drivers/bluetooth/hci_vhci.c
> > > > index 2e302a1..2ed6ab1 100644
> > > > --- a/drivers/bluetooth/hci_vhci.c
> > > > +++ b/drivers/bluetooth/hci_vhci.c
> > > > @@ -41,6 +41,8 @@
> > > >  
> > > >  #define VERSION "1.3"
> > > >  
> > > > +static bool amp;
> > > > +
> > > >  struct vhci_data {
> > > >  	struct hci_dev *hdev;
> > > >  
> > > > @@ -239,6 +241,9 @@ static int vhci_open(struct inode *inode, struct file *file)
> > > >  	hdev->bus = HCI_VIRTUAL;
> > > >  	hdev->driver_data = data;
> > > >  
> > > > +	if (amp)
> > > > +		hdev->dev_type = HCI_AMP;
> > > > +
> > > 
> > > I am thinking about changing lines above to:
> > > 
> > > -       if (amp)
> > > +       if (amp) {
> > >                 hdev->dev_type = HCI_AMP;
> > > +               hdev->amp_type = AMP_CTRL_BLUETOOTH_ONLY;
> > > +       }
> > > 
> > > to indicate that that "the AMP Controller is only used by the Bluetooth
> > > technology and will not be shared with other non-Bluetooth technologies".
> > 
> > this can not be static information at this point. It will change
> > depending on how the WiFi subsystem or WiFi drivers want it to be.
> > 
> > And lets cross the bridge when we get to it. This is too early to figure
> > out what to do here.
> 
> The code only emulates virtual HCI amp_type. The reason why I added it is
> to reply for A2MP Discover Req. If we leave field as 0 it would mean that
> that controller is powered down.
> 
> This value can be read with "read local amp info", I can implement this
> command in hciemu as other option.

as mentioned in the other reply, having it local in A2MP and have hciemu
return the current value seems the correct approach. Everything else
looks like a hack to me.

Regards

Marcel

[PATCHv3 2/3] Bluetooth: Move scope of kernel parameter enable_hs

[Emeltchenko Andrei]

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

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 include/net/bluetooth/hci.h   |    2 ++
 include/net/bluetooth/l2cap.h |    1 -
 2 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 275099f..cdcaac3 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1357,4 +1357,6 @@ struct hci_inquiry_req {
 };
 #define IREQ_CACHE_FLUSH 0x0001
 
+extern int enable_hs;
+
 #endif /* __HCI_H */
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index aeb8676..71df8cb 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -793,7 +793,6 @@ static inline __u8 __ctrl_size(struct l2cap_chan *chan)
 }
 
 extern int disable_ertm;
-extern int enable_hs;
 
 int l2cap_init_sockets(void);
 void l2cap_cleanup_sockets(void);
-- 
1.7.4.1

[PATCHv3 3/3] Bluetooth: Do not set HCI_RAW when HS enabled

[Emeltchenko Andrei]

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

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/hci_core.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 189bada..02a6f15 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -570,8 +570,9 @@ int hci_dev_open(__u16 dev)
 	if (test_bit(HCI_QUIRK_RAW_DEVICE, &hdev->quirks))
 		set_bit(HCI_RAW, &hdev->flags);
 
-	/* Treat all non BR/EDR controllers as raw devices for now */
-	if (hdev->dev_type != HCI_BREDR)
+	/* Treat all non BR/EDR controllers as raw devices if
+	   enable_hs is not set */
+	if (hdev->dev_type != HCI_BREDR && !enable_hs)
 		set_bit(HCI_RAW, &hdev->flags);
 
 	if (hdev->open(hdev)) {
-- 
1.7.4.1

Re: [PATCHv3 3/3] Bluetooth: Do not set HCI_RAW when HS enabled

[Gustavo Padovan]

Hi Andrei,


* Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2011-11-14 12:42:50 +0200]:

> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> Acked-by: Marcel Holtmann <marcel@holtmann.org>
> ---
>  net/bluetooth/hci_core.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)

all three patches were applied, thanks.

	Gustavo