From: Amitkumar Karwar <[email protected]>
Implemented .set_bdaddr handler provided by bluetooth stack for
Marvell devices for public address configuration.
Signed-off-by: Amitkumar Karwar <[email protected]>
Signed-off-by: Bing Zhao <[email protected]>
---
drivers/bluetooth/btusb.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index ed7b33b..9846fc4 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -48,6 +48,7 @@ static struct usb_driver btusb_driver;
#define BTUSB_INTEL 0x100
#define BTUSB_INTEL_BOOT 0x200
#define BTUSB_BCM_PATCHRAM 0x400
+#define BTUSB_MARVELL 0x800
static const struct usb_device_id btusb_table[] = {
/* Generic Bluetooth USB device */
@@ -242,6 +243,10 @@ static const struct usb_device_id blacklist_table[] = {
{ USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL },
{ USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL },
+ /* Marvell device */
+ { USB_DEVICE(0x1286, 0x2044), .driver_info = BTUSB_MARVELL },
+ { USB_DEVICE(0x1286, 0x2046), .driver_info = BTUSB_MARVELL },
+
{ } /* Terminating entry */
};
@@ -1455,6 +1460,29 @@ static int btusb_set_bdaddr_intel(struct hci_dev *hdev, const bdaddr_t *bdaddr)
return 0;
}
+static int btusb_set_bdaddr_marvell(struct hci_dev *hdev,
+ const bdaddr_t *bdaddr)
+{
+ struct sk_buff *skb;
+ u8 buf[8];
+ long ret;
+
+ buf[0] = 0xfe;
+ buf[1] = sizeof(bdaddr_t);
+ bacpy((bdaddr_t *)&buf[2], bdaddr);
+
+ skb = __hci_cmd_sync(hdev, 0xfc22, sizeof(buf), buf, HCI_INIT_TIMEOUT);
+ if (IS_ERR(skb)) {
+ ret = PTR_ERR(skb);
+ BT_ERR("%s: changing Marvell device address failed (%ld)",
+ hdev->name, ret);
+ return ret;
+ }
+ kfree_skb(skb);
+
+ return 0;
+}
+
#define BDADDR_BCM20702A0 (&(bdaddr_t) {{0x00, 0xa0, 0x02, 0x70, 0x20, 0x00}})
static int btusb_setup_bcm_patchram(struct hci_dev *hdev)
@@ -1766,6 +1794,9 @@ static int btusb_probe(struct usb_interface *intf,
hdev->set_bdaddr = btusb_set_bdaddr_intel;
}
+ if (id->driver_info & BTUSB_MARVELL)
+ hdev->set_bdaddr = btusb_set_bdaddr_marvell;
+
if (id->driver_info & BTUSB_INTEL_BOOT)
set_bit(HCI_QUIRK_RAW_DEVICE, &hdev->quirks);
--
1.8.2.3
Hi Marcel,
> > > + ret =3D btmrvl_send_sync_cmd(priv, BT_CMD_SET_BDADDR, buf, sizeof(b=
uf));
> >
> > So I keep wondering why you need the btmrvl_send_sync_cmd here. The HCI=
transport is up and running
> > at this stage and sending vendor command should do the right thing. Sim=
ilar to what you do in btusb
> > driver.
> >
> > You might have explained this to, but I must have clearly not understoo=
d it.
>=20
> We will check if btmrvl_send_sync_cmd() can be replaced by __hci_cmd_sync=
() and get back to you.
__hci_cmd_sync() sets skb->packet_type as 0x01 whereas btmrvl_send_sync_cmd=
() sets it as 0xfe.
But the firmware handles both 0x01 and 0xfe tx packets in similar way. Ther=
efore we can replace btmrvl_send_sync_cmd() with __hci_cmd_sync().
I will send a v2 for this patch shortly. Later on we will send out a cleanu=
p patch for other existing commands that are using btmrvl_send_sync_cmd().
Thanks,
Bing
Hi Marcel,
> > + ret =3D btmrvl_send_sync_cmd(priv, BT_CMD_SET_BDADDR, buf, sizeof(buf=
));
>=20
> So I keep wondering why you need the btmrvl_send_sync_cmd here. The HCI t=
ransport is up and running
> at this stage and sending vendor command should do the right thing. Simil=
ar to what you do in btusb
> driver.
>=20
> You might have explained this to, but I must have clearly not understood =
it.
We will check if btmrvl_send_sync_cmd() can be replaced by __hci_cmd_sync()=
and get back to you.
Thanks,
Bing
>=20
> Regards
>=20
> Marcel
Hi Bing,
> .set_bdaddr handler is implemented for public address configuration.
>
> Signed-off-by: Amitkumar Karwar <[email protected]>
> Signed-off-by: Bing Zhao <[email protected]>
> ---
> drivers/bluetooth/btmrvl_drv.h | 1 +
> drivers/bluetooth/btmrvl_main.c | 15 +++++++++++++++
> 2 files changed, 16 insertions(+)
>
> diff --git a/drivers/bluetooth/btmrvl_drv.h b/drivers/bluetooth/btmrvl_drv.h
> index caf6841..38ad662 100644
> --- a/drivers/bluetooth/btmrvl_drv.h
> +++ b/drivers/bluetooth/btmrvl_drv.h
> @@ -91,6 +91,7 @@ struct btmrvl_private {
>
> /* Vendor specific Bluetooth commands */
> #define BT_CMD_PSCAN_WIN_REPORT_ENABLE 0xFC03
> +#define BT_CMD_SET_BDADDR 0xFC22
> #define BT_CMD_AUTO_SLEEP_MODE 0xFC23
> #define BT_CMD_HOST_SLEEP_CONFIG 0xFC59
> #define BT_CMD_HOST_SLEEP_ENABLE 0xFC5A
> diff --git a/drivers/bluetooth/btmrvl_main.c b/drivers/bluetooth/btmrvl_main.c
> index cc65fd2..1a325fe 100644
> --- a/drivers/bluetooth/btmrvl_main.c
> +++ b/drivers/bluetooth/btmrvl_main.c
> @@ -539,6 +539,20 @@ static int btmrvl_setup(struct hci_dev *hdev)
> return 0;
> }
>
> +static int btmrvl_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
> +{
> + struct btmrvl_private *priv = hci_get_drvdata(hdev);
> + long ret;
> + u8 buf[8];
> +
> + buf[0] = MRVL_VENDOR_PKT;
> + buf[1] = sizeof(bdaddr_t);
> + bacpy((bdaddr_t *)&buf[2], bdaddr);
same thing here. Just use a memcpy.
> + ret = btmrvl_send_sync_cmd(priv, BT_CMD_SET_BDADDR, buf, sizeof(buf));
So I keep wondering why you need the btmrvl_send_sync_cmd here. The HCI transport is up and running at this stage and sending vendor command should do the right thing. Similar to what you do in btusb driver.
You might have explained this to, but I must have clearly not understood it.
Regards
Marcel
Hi Bing,
>>> +static int btusb_set_bdaddr_marvell(struct hci_dev *hdev,
>>> + const bdaddr_t *bdaddr)
>>> +{
>>> + struct sk_buff *skb;
>>> + u8 buf[8];
>>> + long ret;
>>> +
>>> + buf[0] = 0xfe;
>>> + buf[1] = sizeof(bdaddr_t);
>>> + bacpy((bdaddr_t *)&buf[2], bdaddr);
>>
>> you could define a packed struct for this or just use memcpy here instead.
>
> In Amitkumar's original patch, it was like this:
>
> + memcpy((u8 *)buf + 2, bdaddr, sizeof(bdaddr_t));
you do not need the cast:
memcpy(buf + 2, bdaddr, sizeof(bdaddr_t));
>
> I felt that it'd be better to use bacpy so I changed memcpy to bacpy while applying the patch.
> I can change it back to memcpy if it looks ok to you. Otherwise I can define a packed struct.
I am fine with the memcpy actually.
>>> +
>>> + skb = __hci_cmd_sync(hdev, 0xfc22, sizeof(buf), buf, HCI_INIT_TIMEOUT);
>>
>> Is this opcode writing the address into flash and making it permanent or will a reboot restore it to
>> its original address?
>
> A reboot restores it to its original address. I will add this information in v2.
>
>>
>> I am asking because for Ericsson chips this exact command with user_id 0xfe was actually making a
>> permanent change of the BD_ADDR. What we want here is a volatile change of the address.
>
> For Marvell devices it makes a temporary bdaddr change.
Good.
I just know way too much details of all these vendor specific details of the original Bluetooth chips ;)
Regards
Marcel
Hi Marcel,
Thanks for your comments.
> > +static int btusb_set_bdaddr_marvell(struct hci_dev *hdev,
> > + const bdaddr_t *bdaddr)
> > +{
> > + struct sk_buff *skb;
> > + u8 buf[8];
> > + long ret;
> > +
> > + buf[0] =3D 0xfe;
> > + buf[1] =3D sizeof(bdaddr_t);
> > + bacpy((bdaddr_t *)&buf[2], bdaddr);
>=20
> you could define a packed struct for this or just use memcpy here instead=
.
In Amitkumar's original patch, it was like this:
+ memcpy((u8 *)buf + 2, bdaddr, sizeof(bdaddr_t));
I felt that it'd be better to use bacpy so I changed memcpy to bacpy while =
applying the patch.
I can change it back to memcpy if it looks ok to you. Otherwise I can defin=
e a packed struct.
>=20
> > +
> > + skb =3D __hci_cmd_sync(hdev, 0xfc22, sizeof(buf), buf, HCI_INIT_TIMEO=
UT);
>=20
> Is this opcode writing the address into flash and making it permanent or =
will a reboot restore it to
> its original address?
A reboot restores it to its original address. I will add this information i=
n v2.
>=20
> I am asking because for Ericsson chips this exact command with user_id 0x=
fe was actually making a
> permanent change of the BD_ADDR. What we want here is a volatile change o=
f the address.
For Marvell devices it makes a temporary bdaddr change.
Thanks,
Bing
>=20
> Regards
>=20
> Marcel
Hi Bing,
> Implemented .set_bdaddr handler provided by bluetooth stack for
> Marvell devices for public address configuration.
>
> Signed-off-by: Amitkumar Karwar <[email protected]>
> Signed-off-by: Bing Zhao <[email protected]>
> ---
> drivers/bluetooth/btusb.c | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index ed7b33b..9846fc4 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -48,6 +48,7 @@ static struct usb_driver btusb_driver;
> #define BTUSB_INTEL 0x100
> #define BTUSB_INTEL_BOOT 0x200
> #define BTUSB_BCM_PATCHRAM 0x400
> +#define BTUSB_MARVELL 0x800
>
> static const struct usb_device_id btusb_table[] = {
> /* Generic Bluetooth USB device */
> @@ -242,6 +243,10 @@ static const struct usb_device_id blacklist_table[] = {
> { USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL },
> { USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL },
>
> + /* Marvell device */
> + { USB_DEVICE(0x1286, 0x2044), .driver_info = BTUSB_MARVELL },
> + { USB_DEVICE(0x1286, 0x2046), .driver_info = BTUSB_MARVELL },
> +
> { } /* Terminating entry */
> };
>
> @@ -1455,6 +1460,29 @@ static int btusb_set_bdaddr_intel(struct hci_dev *hdev, const bdaddr_t *bdaddr)
> return 0;
> }
>
> +static int btusb_set_bdaddr_marvell(struct hci_dev *hdev,
> + const bdaddr_t *bdaddr)
> +{
> + struct sk_buff *skb;
> + u8 buf[8];
> + long ret;
> +
> + buf[0] = 0xfe;
> + buf[1] = sizeof(bdaddr_t);
> + bacpy((bdaddr_t *)&buf[2], bdaddr);
you could define a packed struct for this or just use memcpy here instead.
> +
> + skb = __hci_cmd_sync(hdev, 0xfc22, sizeof(buf), buf, HCI_INIT_TIMEOUT);
Is this opcode writing the address into flash and making it permanent or will a reboot restore it to its original address?
I am asking because for Ericsson chips this exact command with user_id 0xfe was actually making a permanent change of the BD_ADDR. What we want here is a volatile change of the address.
Regards
Marcel
From: Amitkumar Karwar <[email protected]>
.set_bdaddr handler is implemented for public address configuration.
Signed-off-by: Amitkumar Karwar <[email protected]>
Signed-off-by: Bing Zhao <[email protected]>
---
drivers/bluetooth/btmrvl_drv.h | 1 +
drivers/bluetooth/btmrvl_main.c | 15 +++++++++++++++
2 files changed, 16 insertions(+)
diff --git a/drivers/bluetooth/btmrvl_drv.h b/drivers/bluetooth/btmrvl_drv.h
index caf6841..38ad662 100644
--- a/drivers/bluetooth/btmrvl_drv.h
+++ b/drivers/bluetooth/btmrvl_drv.h
@@ -91,6 +91,7 @@ struct btmrvl_private {
/* Vendor specific Bluetooth commands */
#define BT_CMD_PSCAN_WIN_REPORT_ENABLE 0xFC03
+#define BT_CMD_SET_BDADDR 0xFC22
#define BT_CMD_AUTO_SLEEP_MODE 0xFC23
#define BT_CMD_HOST_SLEEP_CONFIG 0xFC59
#define BT_CMD_HOST_SLEEP_ENABLE 0xFC5A
diff --git a/drivers/bluetooth/btmrvl_main.c b/drivers/bluetooth/btmrvl_main.c
index cc65fd2..1a325fe 100644
--- a/drivers/bluetooth/btmrvl_main.c
+++ b/drivers/bluetooth/btmrvl_main.c
@@ -539,6 +539,20 @@ static int btmrvl_setup(struct hci_dev *hdev)
return 0;
}
+static int btmrvl_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
+{
+ struct btmrvl_private *priv = hci_get_drvdata(hdev);
+ long ret;
+ u8 buf[8];
+
+ buf[0] = MRVL_VENDOR_PKT;
+ buf[1] = sizeof(bdaddr_t);
+ bacpy((bdaddr_t *)&buf[2], bdaddr);
+ ret = btmrvl_send_sync_cmd(priv, BT_CMD_SET_BDADDR, buf, sizeof(buf));
+
+ return 0;
+}
+
/*
* This function handles the event generated by firmware, rx data
* received from firmware, and tx data sent from kernel.
@@ -632,6 +646,7 @@ int btmrvl_register_hdev(struct btmrvl_private *priv)
hdev->flush = btmrvl_flush;
hdev->send = btmrvl_send_frame;
hdev->setup = btmrvl_setup;
+ hdev->set_bdaddr = btmrvl_set_bdaddr;
hdev->dev_type = priv->btmrvl_dev.dev_type;
--
1.8.2.3