[PATCH 0/2] Add API to retrieve the Bluetooth Device Address (BD_ADDR)

[PATCH 0/2] Add API to retrieve the Bluetooth Device Address (BD_ADDR)

[Matthias Kaehlcke]

On some systems the Bluetooth Device Address (BD_ADDR) isn't stored
on the Bluetooth chip itself. One way to configure the BD address is
through the device tree. The btqcomsmd driver is an example, it can
read the BD address from the DT property 'local-bd-address'. It is
also planned to extend the hci_qca driver to support setting the BD
address through the DT.

To avoid redundant open-coded reading of 'local-bd-address' and error
handling this series adds an API to retrieve the BD address of a device
and adapts the btqcomsmd driver to use this API.

Matthias Kaehlcke (2):
  device property: Add device_get_bd_address() and
    fwnode_get_bd_address()
  Bluetooth: btqcomsmd: Get the BD address with device_get_bd_address()

 drivers/base/property.c       | 49 +++++++++++++++++++++++++++++++++++
 drivers/bluetooth/btqcomsmd.c |  5 ++--
 include/linux/property.h      |  4 +++
 3 files changed, 55 insertions(+), 3 deletions(-)

-- 
2.19.0.444.g18242da7ef-goog

[PATCH 1/2] device property: Add device_get_bd_address() and fwnode_get_bd_address()

[Matthias Kaehlcke]

Provide an API for Bluetooth drivers to retrieve the Bluetooth Device
address (BD_ADDR) for a device. If the device node has a property
'local-bd-address' the BD address is read from this property.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 drivers/base/property.c  | 49 ++++++++++++++++++++++++++++++++++++++++
 include/linux/property.h |  4 ++++
 2 files changed, 53 insertions(+)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 240ab5230ff6..8fe546b9805a 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -18,6 +18,8 @@
 #include <linux/etherdevice.h>
 #include <linux/phy.h>
 
+#define BD_ADDR_LEN	6
+
 struct property_set {
 	struct device *dev;
 	struct fwnode_handle fwnode;
@@ -1315,6 +1317,53 @@ void *device_get_mac_address(struct device *dev, char *addr, int alen)
 }
 EXPORT_SYMBOL(device_get_mac_address);
 
+/**
+ * fwnode_get_bd_address - Get the Bluetooth Device Address (BD_ADDR) from the
+ *                         firmware node
+ * @fwnode:	Pointer to the firmware node
+ * @addr:	Address of buffer to store the BD address in
+ * @alen:	Length of the buffer pointed to by addr, should be BD_ADDR_LEN
+ *
+ * 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.
+ */
+int fwnode_get_bd_address(struct fwnode_handle *fwnode, u8 *addr, int alen)
+{
+	u8 buf[BD_ADDR_LEN];
+	int ret;
+
+	if (alen != BD_ADDR_LEN)
+		return -EINVAL;
+
+	ret = fwnode_property_read_u8_array(fwnode, "local-bd-address",
+					    buf, alen);
+	if (ret < 0)
+		return ret;
+	if (is_zero_ether_addr(buf))
+		return -ENODATA;
+
+	memcpy(addr, buf, BD_ADDR_LEN);
+
+	return 0;
+}
+EXPORT_SYMBOL(fwnode_get_bd_address);
+
+/**
+ * device_get_bd_address - Get the Bluetooth Device Address (BD_ADDR) for a
+ *                         given device
+ * @dev:	Pointer to the device
+ * @addr:	Address of buffer to store the BD address in
+ * @alen:	Length of the buffer pointed to by addr, should be BD_ADDR_LEN
+ */
+int device_get_bd_address(struct device *dev, u8 *addr, int alen)
+{
+	return fwnode_get_bd_address(dev_fwnode(dev), addr, alen);
+}
+EXPORT_SYMBOL(device_get_bd_address);
+
 /**
  * fwnode_irq_get - Get IRQ directly from a fwnode
  * @fwnode:	Pointer to the firmware node
diff --git a/include/linux/property.h b/include/linux/property.h
index ac8a1ebc4c1b..4e2f1b276f4f 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -287,9 +287,13 @@ int device_get_phy_mode(struct device *dev);
 
 void *device_get_mac_address(struct device *dev, char *addr, int alen);
 
+int device_get_bd_address(struct device *dev, u8 *addr, int alen);
+
 int fwnode_get_phy_mode(struct fwnode_handle *fwnode);
 void *fwnode_get_mac_address(struct fwnode_handle *fwnode,
 			     char *addr, int alen);
+int fwnode_get_bd_address(struct fwnode_handle *fwnode,
+			  u8 *addr, int alen);
 struct fwnode_handle *fwnode_graph_get_next_endpoint(
 	const struct fwnode_handle *fwnode, struct fwnode_handle *prev);
 struct fwnode_handle *
-- 
2.19.0.444.g18242da7ef-goog

Re: [PATCH 1/2] device property: Add device_get_bd_address() and fwnode_get_bd_address()

[Greg Kroah-Hartman]

On Thu, Sep 20, 2018 at 03:34:35PM -0700, Matthias Kaehlcke wrote:
> +EXPORT_SYMBOL(fwnode_get_bd_address);

EXPORT_SYMBOL_GPL()?  That matches the majority in this file.

thanks,

greg k-h

Re: [PATCH 1/2] device property: Add device_get_bd_address() and fwnode_get_bd_address()

[Matthias Kaehlcke]

On Fri, Sep 21, 2018 at 06:26:40AM +0200, Greg Kroah-Hartman wrote:
> On Thu, Sep 20, 2018 at 03:34:35PM -0700, Matthias Kaehlcke wrote:
> > +EXPORT_SYMBOL(fwnode_get_bd_address);
> 
> EXPORT_SYMBOL_GPL()?  That matches the majority in this file.

Sounds good

Thanks for the review!

m.

Re: [PATCH 1/2] device property: Add device_get_bd_address() and fwnode_get_bd_address()

[Sakari Ailus]

Hi Matthias,

On Thu, Sep 20, 2018 at 03:34:35PM -0700, Matthias Kaehlcke wrote:
> Provide an API for Bluetooth drivers to retrieve the Bluetooth Device
> address (BD_ADDR) for a device. If the device node has a property
> 'local-bd-address' the BD address is read from this property.
> 
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  drivers/base/property.c  | 49 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/property.h |  4 ++++
>  2 files changed, 53 insertions(+)
> 
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 240ab5230ff6..8fe546b9805a 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -18,6 +18,8 @@
>  #include <linux/etherdevice.h>
>  #include <linux/phy.h>
>  
> +#define BD_ADDR_LEN	6
> +
>  struct property_set {
>  	struct device *dev;
>  	struct fwnode_handle fwnode;
> @@ -1315,6 +1317,53 @@ void *device_get_mac_address(struct device *dev, char *addr, int alen)
>  }
>  EXPORT_SYMBOL(device_get_mac_address);
>  
> +/**
> + * fwnode_get_bd_address - Get the Bluetooth Device Address (BD_ADDR) from the
> + *                         firmware node
> + * @fwnode:	Pointer to the firmware node
> + * @addr:	Address of buffer to store the BD address in
> + * @alen:	Length of the buffer pointed to by addr, should be BD_ADDR_LEN
> + *
> + * 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.
> + */
> +int fwnode_get_bd_address(struct fwnode_handle *fwnode, u8 *addr, int alen)

How about using bdaddr_t instead? The original caller was casting it to u8
probably just because there was no function specifically for this purpose.

I wonder if this also should be made depend on CONFIG_BT. The fwnode
framework is unconditionally a part of the kernel and it's not very big at
the moment but adding stuff little by little will add up eventually.

> +{
> +	u8 buf[BD_ADDR_LEN];
> +	int ret;
> +
> +	if (alen != BD_ADDR_LEN)
> +		return -EINVAL;
> +
> +	ret = fwnode_property_read_u8_array(fwnode, "local-bd-address",
> +					    buf, alen);
> +	if (ret < 0)
> +		return ret;
> +	if (is_zero_ether_addr(buf))
> +		return -ENODATA;
> +
> +	memcpy(addr, buf, BD_ADDR_LEN);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(fwnode_get_bd_address);
> +
> +/**
> + * device_get_bd_address - Get the Bluetooth Device Address (BD_ADDR) for a
> + *                         given device
> + * @dev:	Pointer to the device
> + * @addr:	Address of buffer to store the BD address in
> + * @alen:	Length of the buffer pointed to by addr, should be BD_ADDR_LEN
> + */
> +int device_get_bd_address(struct device *dev, u8 *addr, int alen)
> +{
> +	return fwnode_get_bd_address(dev_fwnode(dev), addr, alen);
> +}
> +EXPORT_SYMBOL(device_get_bd_address);
> +
>  /**
>   * fwnode_irq_get - Get IRQ directly from a fwnode
>   * @fwnode:	Pointer to the firmware node
> diff --git a/include/linux/property.h b/include/linux/property.h
> index ac8a1ebc4c1b..4e2f1b276f4f 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -287,9 +287,13 @@ int device_get_phy_mode(struct device *dev);
>  
>  void *device_get_mac_address(struct device *dev, char *addr, int alen);
>  
> +int device_get_bd_address(struct device *dev, u8 *addr, int alen);
> +
>  int fwnode_get_phy_mode(struct fwnode_handle *fwnode);
>  void *fwnode_get_mac_address(struct fwnode_handle *fwnode,
>  			     char *addr, int alen);
> +int fwnode_get_bd_address(struct fwnode_handle *fwnode,
> +			  u8 *addr, int alen);

This would be a good opportunity to add a newline here, and avoid adding
one after device_get_mac_address().

>  struct fwnode_handle *fwnode_graph_get_next_endpoint(
>  	const struct fwnode_handle *fwnode, struct fwnode_handle *prev);
>  struct fwnode_handle *

-- 
Regards,

Sakari Ailus
sakari.ailus@linux.intel.com

Re: [PATCH 1/2] device property: Add device_get_bd_address() and fwnode_get_bd_address()

[Andy Shevchenko]

On Fri, Sep 21, 2018 at 10:16 AM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Matthias,
>
> On Thu, Sep 20, 2018 at 03:34:35PM -0700, Matthias Kaehlcke wrote:
> > Provide an API for Bluetooth drivers to retrieve the Bluetooth Device
> > address (BD_ADDR) for a device. If the device node has a property
> > 'local-bd-address' the BD address is read from this property.
> >

I have nothing to object for the idea and agree fully on Sakari's comments.

After addressing them, fill free to add my

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> >  drivers/base/property.c  | 49 ++++++++++++++++++++++++++++++++++++++++
> >  include/linux/property.h |  4 ++++
> >  2 files changed, 53 insertions(+)
> >
> > diff --git a/drivers/base/property.c b/drivers/base/property.c
> > index 240ab5230ff6..8fe546b9805a 100644
> > --- a/drivers/base/property.c
> > +++ b/drivers/base/property.c
> > @@ -18,6 +18,8 @@
> >  #include <linux/etherdevice.h>
> >  #include <linux/phy.h>
> >
> > +#define BD_ADDR_LEN  6
> > +
> >  struct property_set {
> >       struct device *dev;
> >       struct fwnode_handle fwnode;
> > @@ -1315,6 +1317,53 @@ void *device_get_mac_address(struct device *dev, char *addr, int alen)
> >  }
> >  EXPORT_SYMBOL(device_get_mac_address);
> >
> > +/**
> > + * fwnode_get_bd_address - Get the Bluetooth Device Address (BD_ADDR) from the
> > + *                         firmware node
> > + * @fwnode:  Pointer to the firmware node
> > + * @addr:    Address of buffer to store the BD address in
> > + * @alen:    Length of the buffer pointed to by addr, should be BD_ADDR_LEN
> > + *
> > + * 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.
> > + */
> > +int fwnode_get_bd_address(struct fwnode_handle *fwnode, u8 *addr, int alen)
>
> How about using bdaddr_t instead? The original caller was casting it to u8
> probably just because there was no function specifically for this purpose.
>
> I wonder if this also should be made depend on CONFIG_BT. The fwnode
> framework is unconditionally a part of the kernel and it's not very big at
> the moment but adding stuff little by little will add up eventually.
>
> > +{
> > +     u8 buf[BD_ADDR_LEN];
> > +     int ret;
> > +
> > +     if (alen != BD_ADDR_LEN)
> > +             return -EINVAL;
> > +
> > +     ret = fwnode_property_read_u8_array(fwnode, "local-bd-address",
> > +                                         buf, alen);
> > +     if (ret < 0)
> > +             return ret;
> > +     if (is_zero_ether_addr(buf))
> > +             return -ENODATA;
> > +
> > +     memcpy(addr, buf, BD_ADDR_LEN);
> > +
> > +     return 0;
> > +}
> > +EXPORT_SYMBOL(fwnode_get_bd_address);
> > +
> > +/**
> > + * device_get_bd_address - Get the Bluetooth Device Address (BD_ADDR) for a
> > + *                         given device
> > + * @dev:     Pointer to the device
> > + * @addr:    Address of buffer to store the BD address in
> > + * @alen:    Length of the buffer pointed to by addr, should be BD_ADDR_LEN
> > + */
> > +int device_get_bd_address(struct device *dev, u8 *addr, int alen)
> > +{
> > +     return fwnode_get_bd_address(dev_fwnode(dev), addr, alen);
> > +}
> > +EXPORT_SYMBOL(device_get_bd_address);
> > +
> >  /**
> >   * fwnode_irq_get - Get IRQ directly from a fwnode
> >   * @fwnode:  Pointer to the firmware node
> > diff --git a/include/linux/property.h b/include/linux/property.h
> > index ac8a1ebc4c1b..4e2f1b276f4f 100644
> > --- a/include/linux/property.h
> > +++ b/include/linux/property.h
> > @@ -287,9 +287,13 @@ int device_get_phy_mode(struct device *dev);
> >
> >  void *device_get_mac_address(struct device *dev, char *addr, int alen);
> >
> > +int device_get_bd_address(struct device *dev, u8 *addr, int alen);
> > +
> >  int fwnode_get_phy_mode(struct fwnode_handle *fwnode);
> >  void *fwnode_get_mac_address(struct fwnode_handle *fwnode,
> >                            char *addr, int alen);
> > +int fwnode_get_bd_address(struct fwnode_handle *fwnode,
> > +                       u8 *addr, int alen);
>
> This would be a good opportunity to add a newline here, and avoid adding
> one after device_get_mac_address().
>
> >  struct fwnode_handle *fwnode_graph_get_next_endpoint(
> >       const struct fwnode_handle *fwnode, struct fwnode_handle *prev);
> >  struct fwnode_handle *
>
> --
> Regards,
>
> Sakari Ailus
> sakari.ailus@linux.intel.com



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] device property: Add device_get_bd_address() and fwnode_get_bd_address()

[Matthias Kaehlcke]

Hi Sakari,

On Fri, Sep 21, 2018 at 10:13:12AM +0300, Sakari Ailus wrote:
> Hi Matthias,
> 
> On Thu, Sep 20, 2018 at 03:34:35PM -0700, Matthias Kaehlcke wrote:
> > Provide an API for Bluetooth drivers to retrieve the Bluetooth Device
> > address (BD_ADDR) for a device. If the device node has a property
> > 'local-bd-address' the BD address is read from this property.
> > 
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> >  drivers/base/property.c  | 49 ++++++++++++++++++++++++++++++++++++++++
> >  include/linux/property.h |  4 ++++
> >  2 files changed, 53 insertions(+)
> > 
> > diff --git a/drivers/base/property.c b/drivers/base/property.c
> > index 240ab5230ff6..8fe546b9805a 100644
> > --- a/drivers/base/property.c
> > +++ b/drivers/base/property.c
> > @@ -18,6 +18,8 @@
> >  #include <linux/etherdevice.h>
> >  #include <linux/phy.h>
> >  
> > +#define BD_ADDR_LEN	6
> > +
> >  struct property_set {
> >  	struct device *dev;
> >  	struct fwnode_handle fwnode;
> > @@ -1315,6 +1317,53 @@ void *device_get_mac_address(struct device *dev, char *addr, int alen)
> >  }
> >  EXPORT_SYMBOL(device_get_mac_address);
> >  
> > +/**
> > + * fwnode_get_bd_address - Get the Bluetooth Device Address (BD_ADDR) from the
> > + *                         firmware node
> > + * @fwnode:	Pointer to the firmware node
> > + * @addr:	Address of buffer to store the BD address in
> > + * @alen:	Length of the buffer pointed to by addr, should be BD_ADDR_LEN
> > + *
> > + * 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.
> > + */
> > +int fwnode_get_bd_address(struct fwnode_handle *fwnode, u8 *addr, int alen)
> 
> How about using bdaddr_t instead? The original caller was casting it to u8
> probably just because there was no function specifically for this purpose.

I was considering that, but wasn't sure if it would be 'correct' to
include net/bluetooth/bluetooth.h (with the typdef of bdaddr_t) from
property.h since it is very frugal on headers and complex types.

That said I'd also prefer to use bdaddr_t if the API isn't limited to
basic data types on purpose.

> I wonder if this also should be made depend on CONFIG_BT. The fwnode
> framework is unconditionally a part of the kernel and it's not very big at
> the moment but adding stuff little by little will add up eventually.

Sounds good!

> > +{
> > +	u8 buf[BD_ADDR_LEN];
> > +	int ret;
> > +
> > +	if (alen != BD_ADDR_LEN)
> > +		return -EINVAL;
> > +
> > +	ret = fwnode_property_read_u8_array(fwnode, "local-bd-address",
> > +					    buf, alen);
> > +	if (ret < 0)
> > +		return ret;
> > +	if (is_zero_ether_addr(buf))
> > +		return -ENODATA;
> > +
> > +	memcpy(addr, buf, BD_ADDR_LEN);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(fwnode_get_bd_address);
> > +
> > +/**
> > + * device_get_bd_address - Get the Bluetooth Device Address (BD_ADDR) for a
> > + *                         given device
> > + * @dev:	Pointer to the device
> > + * @addr:	Address of buffer to store the BD address in
> > + * @alen:	Length of the buffer pointed to by addr, should be BD_ADDR_LEN
> > + */
> > +int device_get_bd_address(struct device *dev, u8 *addr, int alen)
> > +{
> > +	return fwnode_get_bd_address(dev_fwnode(dev), addr, alen);
> > +}
> > +EXPORT_SYMBOL(device_get_bd_address);
> > +
> >  /**
> >   * fwnode_irq_get - Get IRQ directly from a fwnode
> >   * @fwnode:	Pointer to the firmware node
> > diff --git a/include/linux/property.h b/include/linux/property.h
> > index ac8a1ebc4c1b..4e2f1b276f4f 100644
> > --- a/include/linux/property.h
> > +++ b/include/linux/property.h
> > @@ -287,9 +287,13 @@ int device_get_phy_mode(struct device *dev);
> >  
> >  void *device_get_mac_address(struct device *dev, char *addr, int alen);
> >  
> > +int device_get_bd_address(struct device *dev, u8 *addr, int alen);
> > +
> >  int fwnode_get_phy_mode(struct fwnode_handle *fwnode);
> >  void *fwnode_get_mac_address(struct fwnode_handle *fwnode,
> >  			     char *addr, int alen);
> > +int fwnode_get_bd_address(struct fwnode_handle *fwnode,
> > +			  u8 *addr, int alen);
> 
> This would be a good opportunity to add a newline here, and avoid adding
> one after device_get_mac_address().

Will do

Thanks for the review!

[PATCH 2/2] Bluetooth: btqcomsmd: Get the BD address with device_get_bd_address()

[Matthias Kaehlcke]

Use the new API to get the BD address instead of reading it directly
from the device tree.

Also remove an unncessary pair of braces in the same area of code.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 drivers/bluetooth/btqcomsmd.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/btqcomsmd.c b/drivers/bluetooth/btqcomsmd.c
index 7df3eed1ef5e..1f2de6aeb3dd 100644
--- a/drivers/bluetooth/btqcomsmd.c
+++ b/drivers/bluetooth/btqcomsmd.c
@@ -172,11 +172,10 @@ static int btqcomsmd_probe(struct platform_device *pdev)
 	/* 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))) {
+	if (!device_get_bd_address(&pdev->dev, (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)
-- 
2.19.0.444.g18242da7ef-goog

Re: [PATCH 0/2] Add API to retrieve the Bluetooth Device Address (BD_ADDR)

[Sinan Kaya]

On 9/20/2018 6:34 PM, Matthias Kaehlcke wrote:
> On some systems the Bluetooth Device Address (BD_ADDR) isn't stored
> on the Bluetooth chip itself. One way to configure the BD address is
> through the device tree. The btqcomsmd driver is an example, it can
> read the BD address from the DT property 'local-bd-address'. It is
> also planned to extend the hci_qca driver to support setting the BD
> address through the DT.
> 
> To avoid redundant open-coded reading of 'local-bd-address' and error
> handling this series adds an API to retrieve the BD address of a device
> and adapts the btqcomsmd driver to use this API.

I don't think you have got enough number of line savings to make this
code to be folded into property.c

Not my call but seems redundant IMO.

Re: [PATCH 0/2] Add API to retrieve the Bluetooth Device Address (BD_ADDR)

[Matthias Kaehlcke]

On Thu, Sep 20, 2018 at 06:45:58PM -0400, Sinan Kaya wrote:
> On 9/20/2018 6:34 PM, Matthias Kaehlcke wrote:
> > On some systems the Bluetooth Device Address (BD_ADDR) isn't stored
> > on the Bluetooth chip itself. One way to configure the BD address is
> > through the device tree. The btqcomsmd driver is an example, it can
> > read the BD address from the DT property 'local-bd-address'. It is
> > also planned to extend the hci_qca driver to support setting the BD
> > address through the DT.
> > 
> > To avoid redundant open-coded reading of 'local-bd-address' and error
> > handling this series adds an API to retrieve the BD address of a device
> > and adapts the btqcomsmd driver to use this API.
> 
> I don't think you have got enough number of line savings to make this
> code to be folded into property.c

Most of the new lines are documentation ;-)

> Not my call but seems redundant IMO.

Certainly true if this was the only driver. However another one will
follow soon and 'local-bd-address' is the official binding, so it's
not unlikely there will be more.

Also fwnode_get_bd_address() handles the case where the property
exists but is all zeros which adds a few LOC, which isn't handled by
the btqcomsmd driver.

Cheers

Matthias

Re: [PATCH 0/2] Add API to retrieve the Bluetooth Device Address (BD_ADDR)

[Sinan Kaya]

On 9/20/2018 7:01 PM, Matthias Kaehlcke wrote:
> Certainly true if this was the only driver. However another one will
> follow soon and 'local-bd-address' is the official binding, so it's
> not unlikely there will be more.

I was missing this information.
Is there a way to convert existing users to this if any as well?

Re: [PATCH 0/2] Add API to retrieve the Bluetooth Device Address (BD_ADDR)

[Matthias Kaehlcke]

On Thu, Sep 20, 2018 at 07:03:15PM -0400, Sinan Kaya wrote:
> On 9/20/2018 7:01 PM, Matthias Kaehlcke wrote:
> > Certainly true if this was the only driver. However another one will
> > follow soon and 'local-bd-address' is the official binding, so it's
> > not unlikely there will be more.
> 
> I was missing this information.

https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/net/bluetooth.txt

> Is there a way to convert existing users to this if any as well?

It depends.

If the existing users use a *really* reasonable name for the property
and use the same format it could be an option to add it to the lookup
besides 'local-bd-address', similar to what fwnode_get_mac_address()
does.

If the existing users populate a custom property with the BD address
in the bootloader you could roll out a bootloader change. You'd have
to make sure that bootloader and kernel match. The bootloader could
still populate the custom property to be compatible with 'old'
kernels.

Re: [PATCH 0/2] Add API to retrieve the Bluetooth Device Address (BD_ADDR)

[Sinan Kaya]

On 9/20/2018 7:19 PM, Matthias Kaehlcke wrote:
> If the existing users populate a custom property with the BD address
> in the bootloader you could roll out a bootloader change. You'd have
> to make sure that bootloader and kernel match. The bootloader could
> still populate the custom property to be compatible with 'old'
> kernels.

If there is no standard about the name of the property, this probably
doesn't belong to property.c but should rather be hosted in bluetooth
directory for consumption among the bluetooth drivers.

Re: [PATCH 0/2] Add API to retrieve the Bluetooth Device Address (BD_ADDR)

[Matthias Kaehlcke]

On Fri, Sep 21, 2018 at 01:25:45AM -0400, Sinan Kaya wrote:
> On 9/20/2018 7:19 PM, Matthias Kaehlcke wrote:
> > If the existing users populate a custom property with the BD address
> > in the bootloader you could roll out a bootloader change. You'd have
> > to make sure that bootloader and kernel match. The bootloader could
> > still populate the custom property to be compatible with 'old'
> > kernels.
> 
> If there is no standard about the name of the property, this probably
> doesn't belong to property.c but should rather be hosted in bluetooth
> directory for consumption among the bluetooth drivers.

Well, there is a generic binding
(https://elixir.bootlin.com/linux/v4.18/source/Documentation/devicetree/bindings/net/bluetooth.txt),
the fact that there are drivers that aren't adhering to it doesn't
change that.

Re: [PATCH 0/2] Add API to retrieve the Bluetooth Device Address (BD_ADDR)

[Sinan Kaya]

On 9/21/2018 12:45 PM, Matthias Kaehlcke wrote:
> Well, there is a generic binding
> (https://elixir.bootlin.com/linux/v4.18/source/Documentation/devicetree/bindings/net/bluetooth.txt),
> the fact that there are drivers that aren't adhering to it doesn't
> change that.

OK. If there is some soft of a "standard" naming convention, we should be good.
Otherwise, as Sakari also raised the concern; common code becomes a dumping
ground for drivers.