linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RESEND 0/3] Add quirk for reading BD_ADDR from fwnode property
@ 2019-01-31 22:10 Matthias Kaehlcke
  2019-01-31 22:10 ` [PATCH RESEND 1/3] Bluetooth: " Matthias Kaehlcke
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Matthias Kaehlcke @ 2019-01-31 22:10 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg, David S . Miller, Loic Poulain
  Cc: linux-bluetooth, linux-kernel, netdev, Balakrishna Godavarthi,
	Matthias Kaehlcke

[initial post: https://lore.kernel.org/patchwork/cover/1028184/]

On some systems the Bluetooth Device Address (BD_ADDR) isn't stored
on the Bluetooth chip itself. One way to configure the address is
through the device tree (patched in by the bootloader). The btqcomsmd
driver is an example, it can read the address from the DT property
'local-bd-address'.

To avoid redundant open-coded reading of 'local-bd-address' and error
handling this series adds the quirk HCI_QUIRK_USE_BDADDR_PROPERTY to
retrieve the BD address of a device from the DT and adapts the
btqcomsmd and hci_qca drivers to use this quirk.

Matthias Kaehlcke (3):
  Bluetooth: Add quirk for reading BD_ADDR from fwnode property
  Bluetooth: btqcomsmd: use HCI_QUIRK_USE_BDADDR_PROPERTY
  Bluetooth: hci_qca: Set HCI_QUIRK_USE_BDADDR_PROPERTY for wcn3990

 drivers/bluetooth/btqcomsmd.c | 29 +++--------------------
 drivers/bluetooth/hci_qca.c   |  1 +
 include/net/bluetooth/hci.h   | 12 ++++++++++
 net/bluetooth/hci_core.c      | 43 +++++++++++++++++++++++++++++++++++
 net/bluetooth/mgmt.c          |  6 +++--
 5 files changed, 63 insertions(+), 28 deletions(-)

-- 
2.20.1.495.gaa96b0ce6b-goog


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH RESEND 1/3] Bluetooth: Add quirk for reading BD_ADDR from fwnode property
  2019-01-31 22:10 [PATCH RESEND 0/3] Add quirk for reading BD_ADDR from fwnode property Matthias Kaehlcke
@ 2019-01-31 22:10 ` Matthias Kaehlcke
  2019-01-31 22:10 ` [PATCH RESEND 2/3] Bluetooth: btqcomsmd: use HCI_QUIRK_USE_BDADDR_PROPERTY Matthias Kaehlcke
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Matthias Kaehlcke @ 2019-01-31 22:10 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg, David S . Miller, Loic Poulain
  Cc: linux-bluetooth, linux-kernel, netdev, Balakrishna Godavarthi,
	Matthias Kaehlcke

Add HCI_QUIRK_USE_BDADDR_PROPERTY to allow controllers to retrieve
the public Bluetooth address from the firmware node property
'local-bd-address'. If quirk is set and the property does not exist
or is invalid the controller is marked as unconfigured.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Balakrishna Godavarthi <bgodavar@codeaurora.org>
Tested-by: Balakrishna Godavarthi <bgodavar@codeaurora.org>
---
Changes in v3:
- return no value from hci_dev_get_bd_addr_from_property() since
  currently nobody uses it anyway
- use bacpy() in hci_dev_get_bd_addr_from_property()
- return -EADDRNOTAVAIL if no BD_ADDR is configured or if the driver
  has no ->set_bdaddr

Changes in v2:
- added check for return value of ->setup()
- only read BD_ADDR from the property if it isn't assigned yet. This
  is needed to support configuration from user space
- refactored the branch of the new quirk to get rid of 'bd_addr_set'
- added 'Reviewed-by: Balakrishna Godavarthi <bgodavar@codeaurora.org>' tag
---
 include/net/bluetooth/hci.h | 12 +++++++++++
 net/bluetooth/hci_core.c    | 43 +++++++++++++++++++++++++++++++++++++
 net/bluetooth/mgmt.c        |  6 ++++--
 3 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index c36dc1e20556a..fbba43e9bef5b 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -158,6 +158,18 @@ enum {
 	 */
 	HCI_QUIRK_INVALID_BDADDR,
 
+	/* When this quirk is set, the public Bluetooth address
+	 * initially reported by HCI Read BD Address command
+	 * is considered invalid. The public BD Address can be
+	 * specified in the fwnode property 'local-bd-address'.
+	 * If this property does not exist or is invalid controller
+	 * configuration is required before this device can be used.
+	 *
+	 * This quirk can be set before hci_register_dev is called or
+	 * during the hdev->setup vendor callback.
+	 */
+	HCI_QUIRK_USE_BDADDR_PROPERTY,
+
 	/* When this quirk is set, the duplicate filtering during
 	 * scanning is based on Bluetooth devices addresses. To allow
 	 * RSSI based updates, restart scanning if needed.
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 7352fe85674be..3ccfa351b87bb 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -30,6 +30,7 @@
 #include <linux/rfkill.h>
 #include <linux/debugfs.h>
 #include <linux/crypto.h>
+#include <linux/property.h>
 #include <asm/unaligned.h>
 
 #include <net/bluetooth/bluetooth.h>
@@ -1355,6 +1356,32 @@ int hci_inquiry(void __user *arg)
 	return err;
 }
 
+/**
+ * hci_dev_get_bd_addr_from_property - Get the Bluetooth Device Address
+ *				       (BD_ADDR) for a HCI device from
+ *				       a firmware node property.
+ * @hdev:	The HCI device
+ *
+ * Search the firmware node for 'local-bd-address'.
+ *
+ * All-zero BD addresses are rejected, because those could be properties
+ * that exist in the firmware tables, but were not updated by the firmware. For
+ * example, the DTS could define 'local-bd-address', with zero BD addresses.
+ */
+static void hci_dev_get_bd_addr_from_property(struct hci_dev *hdev)
+{
+	struct fwnode_handle *fwnode = dev_fwnode(hdev->dev.parent);
+	bdaddr_t ba;
+	int ret;
+
+	ret = fwnode_property_read_u8_array(fwnode, "local-bd-address",
+					    (u8 *)&ba, sizeof(ba));
+	if (ret < 0 || !bacmp(&ba, BDADDR_ANY))
+		return;
+
+	bacpy(&hdev->public_addr, &ba);
+}
+
 static int hci_dev_do_open(struct hci_dev *hdev)
 {
 	int ret = 0;
@@ -1422,6 +1449,22 @@ static int hci_dev_do_open(struct hci_dev *hdev)
 		if (hdev->setup)
 			ret = hdev->setup(hdev);
 
+		if (ret)
+			goto setup_failed;
+
+		if (test_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks)) {
+			if (!bacmp(&hdev->public_addr, BDADDR_ANY))
+				hci_dev_get_bd_addr_from_property(hdev);
+
+			if (bacmp(&hdev->public_addr, BDADDR_ANY) &&
+			    hdev->set_bdaddr)
+				ret = hdev->set_bdaddr(hdev,
+						       &hdev->public_addr);
+			else
+				ret = -EADDRNOTAVAIL;
+		}
+
+setup_failed:
 		/* The transport driver can set these quirks before
 		 * creating the HCI device or in its setup callback.
 		 *
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index ccce954f81468..fae84353d030f 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -551,7 +551,8 @@ static bool is_configured(struct hci_dev *hdev)
 	    !hci_dev_test_flag(hdev, HCI_EXT_CONFIGURED))
 		return false;
 
-	if (test_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks) &&
+	if ((test_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks) ||
+	     test_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks)) &&
 	    !bacmp(&hdev->public_addr, BDADDR_ANY))
 		return false;
 
@@ -566,7 +567,8 @@ static __le32 get_missing_options(struct hci_dev *hdev)
 	    !hci_dev_test_flag(hdev, HCI_EXT_CONFIGURED))
 		options |= MGMT_OPTION_EXTERNAL_CONFIG;
 
-	if (test_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks) &&
+	if ((test_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks) ||
+	     test_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks)) &&
 	    !bacmp(&hdev->public_addr, BDADDR_ANY))
 		options |= MGMT_OPTION_PUBLIC_ADDRESS;
 
-- 
2.20.1.495.gaa96b0ce6b-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH RESEND 2/3] Bluetooth: btqcomsmd: use HCI_QUIRK_USE_BDADDR_PROPERTY
  2019-01-31 22:10 [PATCH RESEND 0/3] Add quirk for reading BD_ADDR from fwnode property Matthias Kaehlcke
  2019-01-31 22:10 ` [PATCH RESEND 1/3] Bluetooth: " Matthias Kaehlcke
@ 2019-01-31 22:10 ` Matthias Kaehlcke
  2019-01-31 22:10 ` [PATCH RESEND 3/3] Bluetooth: hci_qca: Set HCI_QUIRK_USE_BDADDR_PROPERTY for wcn3990 Matthias Kaehlcke
  2019-02-18 10:48 ` [PATCH RESEND 0/3] Add quirk for reading BD_ADDR from fwnode property Marcel Holtmann
  3 siblings, 0 replies; 6+ messages in thread
From: Matthias Kaehlcke @ 2019-01-31 22:10 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg, David S . Miller, Loic Poulain
  Cc: linux-bluetooth, linux-kernel, netdev, Balakrishna Godavarthi,
	Matthias Kaehlcke

Use the HCI_QUIRK_USE_BDADDR_PROPERTY quirk to let the HCI
core handle the reading of 'local-bd-address'. With this there
is no need to set HCI_QUIRK_INVALID_BDADDR, the case of a
non-existing or invalid fwnode property is handled by the core
code.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Balakrishna Godavarthi <bgodavar@codeaurora.org>
---
Changes in v3:
- none

Changes in v2:
- removed now unused field 'bdaddr' from struct btqcomsmd
- added 'Reviewed-by: Balakrishna Godavarthi <bgodavar@codeaurora.org>' tag
---
 drivers/bluetooth/btqcomsmd.c | 29 +++--------------------------
 1 file changed, 3 insertions(+), 26 deletions(-)

diff --git a/drivers/bluetooth/btqcomsmd.c b/drivers/bluetooth/btqcomsmd.c
index 7df3eed1ef5e9..b3020fab6c8e3 100644
--- a/drivers/bluetooth/btqcomsmd.c
+++ b/drivers/bluetooth/btqcomsmd.c
@@ -28,7 +28,6 @@
 struct btqcomsmd {
 	struct hci_dev *hdev;
 
-	bdaddr_t bdaddr;
 	struct rpmsg_endpoint *acl_channel;
 	struct rpmsg_endpoint *cmd_channel;
 };
@@ -125,23 +124,10 @@ static int btqcomsmd_setup(struct hci_dev *hdev)
 		return PTR_ERR(skb);
 	kfree_skb(skb);
 
-	/* Devices do not have persistent storage for BD address. If no
-	 * BD address has been retrieved during probe, mark the device
-	 * as having an invalid BD address.
+	/* Devices do not have persistent storage for BD address. Retrieve
+	 * it from the firmware node property.
 	 */
-	if (!bacmp(&btq->bdaddr, BDADDR_ANY)) {
-		set_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks);
-		return 0;
-	}
-
-	/* When setting a configured BD address fails, mark the device
-	 * as having an invalid BD address.
-	 */
-	err = qca_set_bdaddr_rome(hdev, &btq->bdaddr);
-	if (err) {
-		set_bit(HCI_QUIRK_INVALID_BDADDR, &hdev->quirks);
-		return 0;
-	}
+	set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
 
 	return 0;
 }
@@ -169,15 +155,6 @@ static int btqcomsmd_probe(struct platform_device *pdev)
 	if (IS_ERR(btq->cmd_channel))
 		return PTR_ERR(btq->cmd_channel);
 
-	/* The local-bd-address property is usually injected by the
-	 * bootloader which has access to the allocated BD address.
-	 */
-	if (!of_property_read_u8_array(pdev->dev.of_node, "local-bd-address",
-				       (u8 *)&btq->bdaddr, sizeof(bdaddr_t))) {
-		dev_info(&pdev->dev, "BD address %pMR retrieved from device-tree",
-			 &btq->bdaddr);
-	}
-
 	hdev = hci_alloc_dev();
 	if (!hdev)
 		return -ENOMEM;
-- 
2.20.1.495.gaa96b0ce6b-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH RESEND 3/3] Bluetooth: hci_qca: Set HCI_QUIRK_USE_BDADDR_PROPERTY for wcn3990
  2019-01-31 22:10 [PATCH RESEND 0/3] Add quirk for reading BD_ADDR from fwnode property Matthias Kaehlcke
  2019-01-31 22:10 ` [PATCH RESEND 1/3] Bluetooth: " Matthias Kaehlcke
  2019-01-31 22:10 ` [PATCH RESEND 2/3] Bluetooth: btqcomsmd: use HCI_QUIRK_USE_BDADDR_PROPERTY Matthias Kaehlcke
@ 2019-01-31 22:10 ` Matthias Kaehlcke
  2019-02-18 10:48 ` [PATCH RESEND 0/3] Add quirk for reading BD_ADDR from fwnode property Marcel Holtmann
  3 siblings, 0 replies; 6+ messages in thread
From: Matthias Kaehlcke @ 2019-01-31 22:10 UTC (permalink / raw)
  To: Marcel Holtmann, Johan Hedberg, David S . Miller, Loic Poulain
  Cc: linux-bluetooth, linux-kernel, netdev, Balakrishna Godavarthi,
	Matthias Kaehlcke

Set quirk for wcn3990 to read BD_ADDR from a firmware node property.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Tested-by: Balakrishna Godavarthi <bgodavar@codeaurora.org>
---
Changes in v3:
- none

Changes in v2:
- patch added to the series
---
 drivers/bluetooth/hci_qca.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index f036c8f98ea33..0535833caa52c 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1193,6 +1193,7 @@ static int qca_setup(struct hci_uart *hu)
 		 * setup for every hci up.
 		 */
 		set_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks);
+		set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks);
 		hu->hdev->shutdown = qca_power_off;
 		ret = qca_wcn3990_init(hu);
 		if (ret)
-- 
2.20.1.495.gaa96b0ce6b-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH RESEND 0/3] Add quirk for reading BD_ADDR from fwnode property
  2019-01-31 22:10 [PATCH RESEND 0/3] Add quirk for reading BD_ADDR from fwnode property Matthias Kaehlcke
                   ` (2 preceding siblings ...)
  2019-01-31 22:10 ` [PATCH RESEND 3/3] Bluetooth: hci_qca: Set HCI_QUIRK_USE_BDADDR_PROPERTY for wcn3990 Matthias Kaehlcke
@ 2019-02-18 10:48 ` Marcel Holtmann
  2019-02-19 19:06   ` Matthias Kaehlcke
  3 siblings, 1 reply; 6+ messages in thread
From: Marcel Holtmann @ 2019-02-18 10:48 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Johan Hedberg, David S. Miller, Loic Poulain, linux-bluetooth,
	linux-kernel, netdev, Balakrishna Godavarthi

Hi Matthias,

> [initial post: https://lore.kernel.org/patchwork/cover/1028184/]
> 
> On some systems the Bluetooth Device Address (BD_ADDR) isn't stored
> on the Bluetooth chip itself. One way to configure the address is
> through the device tree (patched in by the bootloader). The btqcomsmd
> driver is an example, it can read the address from the DT property
> 'local-bd-address'.
> 
> To avoid redundant open-coded reading of 'local-bd-address' and error
> handling this series adds the quirk HCI_QUIRK_USE_BDADDR_PROPERTY to
> retrieve the BD address of a device from the DT and adapts the
> btqcomsmd and hci_qca drivers to use this quirk.
> 
> Matthias Kaehlcke (3):
>  Bluetooth: Add quirk for reading BD_ADDR from fwnode property
>  Bluetooth: btqcomsmd: use HCI_QUIRK_USE_BDADDR_PROPERTY
>  Bluetooth: hci_qca: Set HCI_QUIRK_USE_BDADDR_PROPERTY for wcn3990
> 
> drivers/bluetooth/btqcomsmd.c | 29 +++--------------------
> drivers/bluetooth/hci_qca.c   |  1 +
> include/net/bluetooth/hci.h   | 12 ++++++++++
> net/bluetooth/hci_core.c      | 43 +++++++++++++++++++++++++++++++++++
> net/bluetooth/mgmt.c          |  6 +++--
> 5 files changed, 63 insertions(+), 28 deletions(-)

I am getting compiler warnings when trying to apply this set:

  CC      drivers/bluetooth/btqcomsmd.o
drivers/bluetooth/btqcomsmd.c: In function ‘btqcomsmd_setup’:
drivers/bluetooth/btqcomsmd.c:120:6: warning: unused variable ‘err’ [-Wunused-variable]
  int err;
      ^~~
drivers/bluetooth/btqcomsmd.c:118:20: warning: unused variable ‘btq’ [-Wunused-variable]
  struct btqcomsmd *btq = hci_get_drvdata(hdev);
                    ^~~

Regards

Marcel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RESEND 0/3] Add quirk for reading BD_ADDR from fwnode property
  2019-02-18 10:48 ` [PATCH RESEND 0/3] Add quirk for reading BD_ADDR from fwnode property Marcel Holtmann
@ 2019-02-19 19:06   ` Matthias Kaehlcke
  0 siblings, 0 replies; 6+ messages in thread
From: Matthias Kaehlcke @ 2019-02-19 19:06 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Johan Hedberg, David S. Miller, Loic Poulain, linux-bluetooth,
	linux-kernel, netdev, Balakrishna Godavarthi

Hi Marcel,

On Mon, Feb 18, 2019 at 11:48:21AM +0100, Marcel Holtmann wrote:
> Hi Matthias,
> 
> > [initial post: https://lore.kernel.org/patchwork/cover/1028184/]
> > 
> > On some systems the Bluetooth Device Address (BD_ADDR) isn't stored
> > on the Bluetooth chip itself. One way to configure the address is
> > through the device tree (patched in by the bootloader). The btqcomsmd
> > driver is an example, it can read the address from the DT property
> > 'local-bd-address'.
> > 
> > To avoid redundant open-coded reading of 'local-bd-address' and error
> > handling this series adds the quirk HCI_QUIRK_USE_BDADDR_PROPERTY to
> > retrieve the BD address of a device from the DT and adapts the
> > btqcomsmd and hci_qca drivers to use this quirk.
> > 
> > Matthias Kaehlcke (3):
> >  Bluetooth: Add quirk for reading BD_ADDR from fwnode property
> >  Bluetooth: btqcomsmd: use HCI_QUIRK_USE_BDADDR_PROPERTY
> >  Bluetooth: hci_qca: Set HCI_QUIRK_USE_BDADDR_PROPERTY for wcn3990
> > 
> > drivers/bluetooth/btqcomsmd.c | 29 +++--------------------
> > drivers/bluetooth/hci_qca.c   |  1 +
> > include/net/bluetooth/hci.h   | 12 ++++++++++
> > net/bluetooth/hci_core.c      | 43 +++++++++++++++++++++++++++++++++++
> > net/bluetooth/mgmt.c          |  6 +++--
> > 5 files changed, 63 insertions(+), 28 deletions(-)
> 
> I am getting compiler warnings when trying to apply this set:
> 
>   CC      drivers/bluetooth/btqcomsmd.o
> drivers/bluetooth/btqcomsmd.c: In function ‘btqcomsmd_setup’:
> drivers/bluetooth/btqcomsmd.c:120:6: warning: unused variable ‘err’ [-Wunused-variable]
>   int err;
>       ^~~
> drivers/bluetooth/btqcomsmd.c:118:20: warning: unused variable ‘btq’ [-Wunused-variable]
>   struct btqcomsmd *btq = hci_get_drvdata(hdev);
>                     ^~~

Sorry, I missed that the variables aren't used anymore. I'll send a
new version soon.

Thanks

Matthias

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-02-19 19:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-31 22:10 [PATCH RESEND 0/3] Add quirk for reading BD_ADDR from fwnode property Matthias Kaehlcke
2019-01-31 22:10 ` [PATCH RESEND 1/3] Bluetooth: " Matthias Kaehlcke
2019-01-31 22:10 ` [PATCH RESEND 2/3] Bluetooth: btqcomsmd: use HCI_QUIRK_USE_BDADDR_PROPERTY Matthias Kaehlcke
2019-01-31 22:10 ` [PATCH RESEND 3/3] Bluetooth: hci_qca: Set HCI_QUIRK_USE_BDADDR_PROPERTY for wcn3990 Matthias Kaehlcke
2019-02-18 10:48 ` [PATCH RESEND 0/3] Add quirk for reading BD_ADDR from fwnode property Marcel Holtmann
2019-02-19 19:06   ` Matthias Kaehlcke

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).