* [PATCH v3.1 0/2] Add API to retrieve the Bluetooth Device Address (BD_ADDR) @ 2018-09-25 19:10 Matthias Kaehlcke 2018-09-25 19:10 ` [PATCH v3.1 1/2] device property: Add device_get_bd_address() and fwnode_get_bd_address() Matthias Kaehlcke 2018-09-25 19:10 ` [PATCH v3.1 2/2] Bluetooth: btqcomsmd: Get the BD address with device_get_bd_address() Matthias Kaehlcke 0 siblings, 2 replies; 13+ messages in thread From: Matthias Kaehlcke @ 2018-09-25 19:10 UTC (permalink / raw) To: Greg Kroah-Hartman, Rafael J . Wysocki, Sakari Ailus, Marcin Wojtas, Andy Shevchenko Andy Shevchenko, Sinan Kaya, Marcel Holtmann, Johan Hedberg, Andrew Morton, Pekka Enberg, Masahiro Yamada, Alexey Dobriyan Cc: linux-kernel, linux-bluetooth, Balakrishna Godavarthi, Loic Poulain, Brian Norris, Matthias Kaehlcke (previous v3 post was messed up, resending as v3.1 with the correct stack) 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 | 46 +++++++++++++++++++++++++++++++ drivers/bluetooth/btqcomsmd.c | 4 +-- include/linux/property.h | 18 ++++++++++++ include/linux/types.h | 5 ++++ include/net/bluetooth/bluetooth.h | 5 ---- 5 files changed, 70 insertions(+), 8 deletions(-) -- 2.19.0.605.g01d371f741-goog ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3.1 1/2] device property: Add device_get_bd_address() and fwnode_get_bd_address() 2018-09-25 19:10 [PATCH v3.1 0/2] Add API to retrieve the Bluetooth Device Address (BD_ADDR) Matthias Kaehlcke @ 2018-09-25 19:10 ` Matthias Kaehlcke 2018-09-25 21:33 ` Sakari Ailus 2018-09-26 11:36 ` Heikki Krogerus 2018-09-25 19:10 ` [PATCH v3.1 2/2] Bluetooth: btqcomsmd: Get the BD address with device_get_bd_address() Matthias Kaehlcke 1 sibling, 2 replies; 13+ messages in thread From: Matthias Kaehlcke @ 2018-09-25 19:10 UTC (permalink / raw) To: Greg Kroah-Hartman, Rafael J . Wysocki, Sakari Ailus, Marcin Wojtas, Andy Shevchenko Andy Shevchenko, Sinan Kaya, Marcel Holtmann, Johan Hedberg, Andrew Morton, Pekka Enberg, Masahiro Yamada, Alexey Dobriyan Cc: linux-kernel, linux-bluetooth, Balakrishna Godavarthi, Loic Poulain, Brian Norris, 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. The definition of bdaddr_t is moved to types.h to make it visible in property.h without having to include (the mostly unrelated) bluetooth.h Signed-off-by: Matthias Kaehlcke <mka@chromium.org> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> --- Changes in v3: - move definition of bdaddr_t to types.h to avoid include of bluetooth.h from property.h - add stubs for the new functions Changes in v2: - use bdaddr_t instead of byte pointer + len - use EXPORT_SYMBOL_GPL for the new functions instead of EXPORT_SYMBOL - put new functions inside #if IS_ENABLED(CONFIG_BT) - some new line juggling in property.h - added 'Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>' tag --- drivers/base/property.c | 46 +++++++++++++++++++++++++++++++ include/linux/property.h | 18 ++++++++++++ include/linux/types.h | 5 ++++ include/net/bluetooth/bluetooth.h | 5 ---- 4 files changed, 69 insertions(+), 5 deletions(-) diff --git a/drivers/base/property.c b/drivers/base/property.c index 240ab5230ff6..afe412133188 100644 --- a/drivers/base/property.c +++ b/drivers/base/property.c @@ -1315,6 +1315,52 @@ void *device_get_mac_address(struct device *dev, char *addr, int alen) } EXPORT_SYMBOL(device_get_mac_address); +#if IS_ENABLED(CONFIG_BT) + +/** + * fwnode_get_bd_address - Get the Bluetooth Device Address (BD_ADDR) from the + * firmware node + * @fwnode: Pointer to the firmware node + * @bd_addr: Pointer to struct to store the BD address in + * + * 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, bdaddr_t *bd_addr) +{ + bdaddr_t ba; + int ret; + + ret = fwnode_property_read_u8_array(fwnode, "local-bd-address", + (u8 *)&ba, sizeof(bdaddr_t)); + if (ret < 0) + return ret; + if (is_zero_ether_addr((u8 *)&ba)) + return -ENODATA; + + memcpy(bd_addr, &ba, sizeof(bdaddr_t)); + + return 0; +} +EXPORT_SYMBOL_GPL(fwnode_get_bd_address); + +/** + * device_get_bd_address - Get the Bluetooth Device Address (BD_ADDR) for a + * given device + * @dev: Pointer to the device + * @bd_addr: Pointer to struct to store the BD address in + */ +int device_get_bd_address(struct device *dev, bdaddr_t *bd_addr) +{ + return fwnode_get_bd_address(dev_fwnode(dev), bd_addr); +} +EXPORT_SYMBOL_GPL(device_get_bd_address); + +#endif + /** * 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..5df4e0bd8c83 100644 --- a/include/linux/property.h +++ b/include/linux/property.h @@ -286,10 +286,28 @@ const void *device_get_match_data(struct device *dev); int device_get_phy_mode(struct device *dev); void *device_get_mac_address(struct device *dev, char *addr, int alen); +#if IS_ENABLED(CONFIG_BT) +int device_get_bd_address(struct device *dev, bdaddr_t *bd_addr); +#else +static inline int device_get_bd_address(struct device *dev, + bdaddr_t *bd_addr) +{ + return -ENOTSUPP; +} +#endif int fwnode_get_phy_mode(struct fwnode_handle *fwnode); void *fwnode_get_mac_address(struct fwnode_handle *fwnode, char *addr, int alen); +#if IS_ENABLED(CONFIG_BT) +int fwnode_get_bd_address(struct fwnode_handle *fwnode, bdaddr_t *bd_addr); +#else +static inline int fwnode_get_bd_address(struct fwnode_handle *fwnode, + bdaddr_t *bd_addr) +{ + return -ENOTSUPP; +} +#endif struct fwnode_handle *fwnode_graph_get_next_endpoint( const struct fwnode_handle *fwnode, struct fwnode_handle *prev); struct fwnode_handle * diff --git a/include/linux/types.h b/include/linux/types.h index 9834e90aa010..ff6984a00a3b 100644 --- a/include/linux/types.h +++ b/include/linux/types.h @@ -230,5 +230,10 @@ struct callback_head { typedef void (*rcu_callback_t)(struct rcu_head *head); typedef void (*call_rcu_func_t)(struct rcu_head *head, rcu_callback_t func); +/* Bluetooth Device Address (BD_ADDR) */ +typedef struct { + __u8 b[6]; +} __packed bdaddr_t; + #endif /* __ASSEMBLY__ */ #endif /* _LINUX_TYPES_H */ diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h index ec9d6bc65855..9401c7431a8f 100644 --- a/include/net/bluetooth/bluetooth.h +++ b/include/net/bluetooth/bluetooth.h @@ -190,11 +190,6 @@ static inline const char *state_to_string(int state) return "invalid state"; } -/* BD Address */ -typedef struct { - __u8 b[6]; -} __packed bdaddr_t; - /* BD Address type */ #define BDADDR_BREDR 0x00 #define BDADDR_LE_PUBLIC 0x01 -- 2.19.0.605.g01d371f741-goog ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3.1 1/2] device property: Add device_get_bd_address() and fwnode_get_bd_address() 2018-09-25 19:10 ` [PATCH v3.1 1/2] device property: Add device_get_bd_address() and fwnode_get_bd_address() Matthias Kaehlcke @ 2018-09-25 21:33 ` Sakari Ailus 2018-09-25 23:50 ` Matthias Kaehlcke 2018-09-26 11:36 ` Heikki Krogerus 1 sibling, 1 reply; 13+ messages in thread From: Sakari Ailus @ 2018-09-25 21:33 UTC (permalink / raw) To: Matthias Kaehlcke Cc: Greg Kroah-Hartman, Rafael J . Wysocki, Marcin Wojtas, Andy Shevchenko Andy Shevchenko, Sinan Kaya, Marcel Holtmann, Johan Hedberg, Andrew Morton, Pekka Enberg, Masahiro Yamada, Alexey Dobriyan, linux-kernel, linux-bluetooth, Balakrishna Godavarthi, Loic Poulain, Brian Norris Hi Matthias, On Tue, Sep 25, 2018 at 12:10:13PM -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. > > The definition of bdaddr_t is moved to types.h to make it visible in > property.h without having to include (the mostly unrelated) bluetooth.h > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > --- > Changes in v3: > - move definition of bdaddr_t to types.h to avoid include of > bluetooth.h from property.h > - add stubs for the new functions > > Changes in v2: > - use bdaddr_t instead of byte pointer + len > - use EXPORT_SYMBOL_GPL for the new functions instead of EXPORT_SYMBOL > - put new functions inside #if IS_ENABLED(CONFIG_BT) > - some new line juggling in property.h > - added 'Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>' tag > --- > drivers/base/property.c | 46 +++++++++++++++++++++++++++++++ > include/linux/property.h | 18 ++++++++++++ > include/linux/types.h | 5 ++++ > include/net/bluetooth/bluetooth.h | 5 ---- > 4 files changed, 69 insertions(+), 5 deletions(-) > > diff --git a/drivers/base/property.c b/drivers/base/property.c > index 240ab5230ff6..afe412133188 100644 > --- a/drivers/base/property.c > +++ b/drivers/base/property.c > @@ -1315,6 +1315,52 @@ void *device_get_mac_address(struct device *dev, char *addr, int alen) > } > EXPORT_SYMBOL(device_get_mac_address); > > +#if IS_ENABLED(CONFIG_BT) > + > +/** > + * fwnode_get_bd_address - Get the Bluetooth Device Address (BD_ADDR) from the > + * firmware node > + * @fwnode: Pointer to the firmware node > + * @bd_addr: Pointer to struct to store the BD address in > + * > + * 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, bdaddr_t *bd_addr) > +{ > + bdaddr_t ba; > + int ret; > + > + ret = fwnode_property_read_u8_array(fwnode, "local-bd-address", > + (u8 *)&ba, sizeof(bdaddr_t)); sizeof(ba) > + if (ret < 0) > + return ret; > + if (is_zero_ether_addr((u8 *)&ba)) > + return -ENODATA; > + > + memcpy(bd_addr, &ba, sizeof(bdaddr_t)); How about simply: *bd_addr = ba; Either way, Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com> > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(fwnode_get_bd_address); > + > +/** > + * device_get_bd_address - Get the Bluetooth Device Address (BD_ADDR) for a > + * given device > + * @dev: Pointer to the device > + * @bd_addr: Pointer to struct to store the BD address in > + */ > +int device_get_bd_address(struct device *dev, bdaddr_t *bd_addr) > +{ > + return fwnode_get_bd_address(dev_fwnode(dev), bd_addr); > +} > +EXPORT_SYMBOL_GPL(device_get_bd_address); > + > +#endif > + > /** > * 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..5df4e0bd8c83 100644 > --- a/include/linux/property.h > +++ b/include/linux/property.h > @@ -286,10 +286,28 @@ const void *device_get_match_data(struct device *dev); > int device_get_phy_mode(struct device *dev); > > void *device_get_mac_address(struct device *dev, char *addr, int alen); > +#if IS_ENABLED(CONFIG_BT) > +int device_get_bd_address(struct device *dev, bdaddr_t *bd_addr); > +#else > +static inline int device_get_bd_address(struct device *dev, > + bdaddr_t *bd_addr) > +{ > + return -ENOTSUPP; > +} > +#endif > > int fwnode_get_phy_mode(struct fwnode_handle *fwnode); > void *fwnode_get_mac_address(struct fwnode_handle *fwnode, > char *addr, int alen); > +#if IS_ENABLED(CONFIG_BT) > +int fwnode_get_bd_address(struct fwnode_handle *fwnode, bdaddr_t *bd_addr); > +#else > +static inline int fwnode_get_bd_address(struct fwnode_handle *fwnode, > + bdaddr_t *bd_addr) > +{ > + return -ENOTSUPP; > +} > +#endif > struct fwnode_handle *fwnode_graph_get_next_endpoint( > const struct fwnode_handle *fwnode, struct fwnode_handle *prev); > struct fwnode_handle * > diff --git a/include/linux/types.h b/include/linux/types.h > index 9834e90aa010..ff6984a00a3b 100644 > --- a/include/linux/types.h > +++ b/include/linux/types.h > @@ -230,5 +230,10 @@ struct callback_head { > typedef void (*rcu_callback_t)(struct rcu_head *head); > typedef void (*call_rcu_func_t)(struct rcu_head *head, rcu_callback_t func); > > +/* Bluetooth Device Address (BD_ADDR) */ > +typedef struct { > + __u8 b[6]; > +} __packed bdaddr_t; > + > #endif /* __ASSEMBLY__ */ > #endif /* _LINUX_TYPES_H */ > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h > index ec9d6bc65855..9401c7431a8f 100644 > --- a/include/net/bluetooth/bluetooth.h > +++ b/include/net/bluetooth/bluetooth.h > @@ -190,11 +190,6 @@ static inline const char *state_to_string(int state) > return "invalid state"; > } > > -/* BD Address */ > -typedef struct { > - __u8 b[6]; > -} __packed bdaddr_t; > - > /* BD Address type */ > #define BDADDR_BREDR 0x00 > #define BDADDR_LE_PUBLIC 0x01 -- Kind regards, Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3.1 1/2] device property: Add device_get_bd_address() and fwnode_get_bd_address() 2018-09-25 21:33 ` Sakari Ailus @ 2018-09-25 23:50 ` Matthias Kaehlcke 0 siblings, 0 replies; 13+ messages in thread From: Matthias Kaehlcke @ 2018-09-25 23:50 UTC (permalink / raw) To: Sakari Ailus Cc: Greg Kroah-Hartman, Rafael J . Wysocki, Marcin Wojtas, Andy Shevchenko Andy Shevchenko, Sinan Kaya, Marcel Holtmann, Johan Hedberg, Andrew Morton, Pekka Enberg, Masahiro Yamada, Alexey Dobriyan, linux-kernel, linux-bluetooth, Balakrishna Godavarthi, Loic Poulain, Brian Norris Hi Sakari, thanks for the reviews On Wed, Sep 26, 2018 at 12:33:22AM +0300, Sakari Ailus wrote: > Hi Matthias, > > On Tue, Sep 25, 2018 at 12:10:13PM -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. > > > > The definition of bdaddr_t is moved to types.h to make it visible in > > property.h without having to include (the mostly unrelated) bluetooth.h > > > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org> > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > --- > > Changes in v3: > > - move definition of bdaddr_t to types.h to avoid include of > > bluetooth.h from property.h > > - add stubs for the new functions > > > > Changes in v2: > > - use bdaddr_t instead of byte pointer + len > > - use EXPORT_SYMBOL_GPL for the new functions instead of EXPORT_SYMBOL > > - put new functions inside #if IS_ENABLED(CONFIG_BT) > > - some new line juggling in property.h > > - added 'Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>' tag > > --- > > drivers/base/property.c | 46 +++++++++++++++++++++++++++++++ > > include/linux/property.h | 18 ++++++++++++ > > include/linux/types.h | 5 ++++ > > include/net/bluetooth/bluetooth.h | 5 ---- > > 4 files changed, 69 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/base/property.c b/drivers/base/property.c > > index 240ab5230ff6..afe412133188 100644 > > --- a/drivers/base/property.c > > +++ b/drivers/base/property.c > > @@ -1315,6 +1315,52 @@ void *device_get_mac_address(struct device *dev, char *addr, int alen) > > } > > EXPORT_SYMBOL(device_get_mac_address); > > > > +#if IS_ENABLED(CONFIG_BT) > > + > > +/** > > + * fwnode_get_bd_address - Get the Bluetooth Device Address (BD_ADDR) from the > > + * firmware node > > + * @fwnode: Pointer to the firmware node > > + * @bd_addr: Pointer to struct to store the BD address in > > + * > > + * 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, bdaddr_t *bd_addr) > > +{ > > + bdaddr_t ba; > > + int ret; > > + > > + ret = fwnode_property_read_u8_array(fwnode, "local-bd-address", > > + (u8 *)&ba, sizeof(bdaddr_t)); > > sizeof(ba) > > > + if (ret < 0) > > + return ret; > > + if (is_zero_ether_addr((u8 *)&ba)) > > + return -ENODATA; > > + > > + memcpy(bd_addr, &ba, sizeof(bdaddr_t)); > > How about simply: > > *bd_addr = ba; > > Either way, > > Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com> Thanks I'll address your comments in v4. I'll wait a day or two before re-spinning for if others have additional feedback. Cheers Matthias ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3.1 1/2] device property: Add device_get_bd_address() and fwnode_get_bd_address() 2018-09-25 19:10 ` [PATCH v3.1 1/2] device property: Add device_get_bd_address() and fwnode_get_bd_address() Matthias Kaehlcke 2018-09-25 21:33 ` Sakari Ailus @ 2018-09-26 11:36 ` Heikki Krogerus 2018-09-26 17:12 ` Andy Shevchenko ` (2 more replies) 1 sibling, 3 replies; 13+ messages in thread From: Heikki Krogerus @ 2018-09-26 11:36 UTC (permalink / raw) To: Matthias Kaehlcke Cc: Greg Kroah-Hartman, Rafael J . Wysocki, Sakari Ailus, Marcin Wojtas, Andy Shevchenko Andy Shevchenko, Sinan Kaya, Marcel Holtmann, Johan Hedberg, Andrew Morton, Pekka Enberg, Masahiro Yamada, Alexey Dobriyan, linux-kernel, linux-bluetooth, Balakrishna Godavarthi, Loic Poulain, Brian Norris On Tue, Sep 25, 2018 at 12:10:13PM -0700, Matthias Kaehlcke wrote: > +/** > + * fwnode_get_bd_address - Get the Bluetooth Device Address (BD_ADDR) from the > + * firmware node > + * @fwnode: Pointer to the firmware node > + * @bd_addr: Pointer to struct to store the BD address in > + * > + * 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, bdaddr_t *bd_addr) > +{ > + bdaddr_t ba; > + int ret; > + > + ret = fwnode_property_read_u8_array(fwnode, "local-bd-address", > + (u8 *)&ba, sizeof(bdaddr_t)); > + if (ret < 0) > + return ret; > + if (is_zero_ether_addr((u8 *)&ba)) > + return -ENODATA; > + > + memcpy(bd_addr, &ba, sizeof(bdaddr_t)); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(fwnode_get_bd_address); > + > +/** > + * device_get_bd_address - Get the Bluetooth Device Address (BD_ADDR) for a > + * given device > + * @dev: Pointer to the device > + * @bd_addr: Pointer to struct to store the BD address in > + */ > +int device_get_bd_address(struct device *dev, bdaddr_t *bd_addr) > +{ > + return fwnode_get_bd_address(dev_fwnode(dev), bd_addr); > +} > +EXPORT_SYMBOL_GPL(device_get_bd_address); Let's not fill property.c with framework specific helper functions any more! Those functions are completely bluetooth specific, so they do not belong here. The fact that some other framework already managed to slip their helpers in does not justify others to do the same. Thanks, -- heikki ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3.1 1/2] device property: Add device_get_bd_address() and fwnode_get_bd_address() 2018-09-26 11:36 ` Heikki Krogerus @ 2018-09-26 17:12 ` Andy Shevchenko 2018-09-26 21:03 ` Matthias Kaehlcke 2018-09-27 10:24 ` Marcel Holtmann 2 siblings, 0 replies; 13+ messages in thread From: Andy Shevchenko @ 2018-09-26 17:12 UTC (permalink / raw) To: Krogerus, Heikki Cc: Matthias Kaehlcke, Greg Kroah-Hartman, Rafael J. Wysocki, Sakari Ailus, Marcin Wojtas, Sinan Kaya, Marcel Holtmann, Johan Hedberg, Andrew Morton, Pekka Enberg, Masahiro Yamada, Alexey Dobriyan, Linux Kernel Mailing List, linux-bluetooth, bgodavar, Loic Poulain, Brian Norris On Wed, Sep 26, 2018 at 2:37 PM Heikki Krogerus <heikki.krogerus@linux.intel.com> wrote: > > On Tue, Sep 25, 2018 at 12:10:13PM -0700, Matthias Kaehlcke wrote: > Let's not fill property.c with framework specific helper functions any > more! > > Those functions are completely bluetooth specific, so they do not > belong here. The fact that some other framework already managed to > slip their helpers in does not justify others to do the same. Actually good point. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3.1 1/2] device property: Add device_get_bd_address() and fwnode_get_bd_address() 2018-09-26 11:36 ` Heikki Krogerus 2018-09-26 17:12 ` Andy Shevchenko @ 2018-09-26 21:03 ` Matthias Kaehlcke 2018-09-27 10:24 ` Marcel Holtmann 2 siblings, 0 replies; 13+ messages in thread From: Matthias Kaehlcke @ 2018-09-26 21:03 UTC (permalink / raw) To: Heikki Krogerus Cc: Greg Kroah-Hartman, Rafael J . Wysocki, Sakari Ailus, Marcin Wojtas, Andy Shevchenko Andy Shevchenko, Sinan Kaya, Marcel Holtmann, Johan Hedberg, Andrew Morton, Pekka Enberg, Masahiro Yamada, Alexey Dobriyan, linux-kernel, linux-bluetooth, Balakrishna Godavarthi, Loic Poulain, Brian Norris On Wed, Sep 26, 2018 at 02:36:25PM +0300, Heikki Krogerus wrote: > On Tue, Sep 25, 2018 at 12:10:13PM -0700, Matthias Kaehlcke wrote: > > +/** > > + * fwnode_get_bd_address - Get the Bluetooth Device Address (BD_ADDR) from the > > + * firmware node > > + * @fwnode: Pointer to the firmware node > > + * @bd_addr: Pointer to struct to store the BD address in > > + * > > + * 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, bdaddr_t *bd_addr) > > +{ > > + bdaddr_t ba; > > + int ret; > > + > > + ret = fwnode_property_read_u8_array(fwnode, "local-bd-address", > > + (u8 *)&ba, sizeof(bdaddr_t)); > > + if (ret < 0) > > + return ret; > > + if (is_zero_ether_addr((u8 *)&ba)) > > + return -ENODATA; > > + > > + memcpy(bd_addr, &ba, sizeof(bdaddr_t)); > > + > > + return 0; > > +} > > +EXPORT_SYMBOL_GPL(fwnode_get_bd_address); > > + > > +/** > > + * device_get_bd_address - Get the Bluetooth Device Address (BD_ADDR) for a > > + * given device > > + * @dev: Pointer to the device > > + * @bd_addr: Pointer to struct to store the BD address in > > + */ > > +int device_get_bd_address(struct device *dev, bdaddr_t *bd_addr) > > +{ > > + return fwnode_get_bd_address(dev_fwnode(dev), bd_addr); > > +} > > +EXPORT_SYMBOL_GPL(device_get_bd_address); > > Let's not fill property.c with framework specific helper functions any > more! > > Those functions are completely bluetooth specific, so they do not > belong here. The fact that some other framework already managed to > slip their helpers in does not justify others to do the same. You have a point, I'll see if I can find a better place. Thanks Matthias ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3.1 1/2] device property: Add device_get_bd_address() and fwnode_get_bd_address() 2018-09-26 11:36 ` Heikki Krogerus 2018-09-26 17:12 ` Andy Shevchenko 2018-09-26 21:03 ` Matthias Kaehlcke @ 2018-09-27 10:24 ` Marcel Holtmann 2018-09-27 11:38 ` Heikki Krogerus 2 siblings, 1 reply; 13+ messages in thread From: Marcel Holtmann @ 2018-09-27 10:24 UTC (permalink / raw) To: Heikki Krogerus Cc: Matthias Kaehlcke, Greg Kroah-Hartman, Rafael J . Wysocki, Sakari Ailus, Marcin Wojtas, Andy Shevchenko Andy Shevchenko, Sinan Kaya, Johan Hedberg, Andrew Morton, Pekka Enberg, Masahiro Yamada, Alexey Dobriyan, linux-kernel, linux-bluetooth, Balakrishna Godavarthi, Loic Poulain, Brian Norris Hi Heikki, >> +/** >> + * fwnode_get_bd_address - Get the Bluetooth Device Address (BD_ADDR) from the >> + * firmware node >> + * @fwnode: Pointer to the firmware node >> + * @bd_addr: Pointer to struct to store the BD address in >> + * >> + * 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, bdaddr_t *bd_addr) >> +{ >> + bdaddr_t ba; >> + int ret; >> + >> + ret = fwnode_property_read_u8_array(fwnode, "local-bd-address", >> + (u8 *)&ba, sizeof(bdaddr_t)); >> + if (ret < 0) >> + return ret; >> + if (is_zero_ether_addr((u8 *)&ba)) >> + return -ENODATA; >> + >> + memcpy(bd_addr, &ba, sizeof(bdaddr_t)); >> + >> + return 0; >> +} >> +EXPORT_SYMBOL_GPL(fwnode_get_bd_address); >> + >> +/** >> + * device_get_bd_address - Get the Bluetooth Device Address (BD_ADDR) for a >> + * given device >> + * @dev: Pointer to the device >> + * @bd_addr: Pointer to struct to store the BD address in >> + */ >> +int device_get_bd_address(struct device *dev, bdaddr_t *bd_addr) >> +{ >> + return fwnode_get_bd_address(dev_fwnode(dev), bd_addr); >> +} >> +EXPORT_SYMBOL_GPL(device_get_bd_address); > > Let's not fill property.c with framework specific helper functions any > more! > > Those functions are completely bluetooth specific, so they do not > belong here. The fact that some other framework already managed to > slip their helpers in does not justify others to do the same. so? The firmware guys decided to put MAC addresses and BD addresses into the firmware. So you have to deal with that. Moving this into the Bluetooth subsystem is as pointless. I rather keep the accessor function to firmware specific data in one place and not spread around the whole tree. Especially once this is also provided via ACPI or some other means. I assumed that is what the whole device_get part was suppose to abstract. Regards Marcel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3.1 1/2] device property: Add device_get_bd_address() and fwnode_get_bd_address() 2018-09-27 10:24 ` Marcel Holtmann @ 2018-09-27 11:38 ` Heikki Krogerus 2018-09-27 13:15 ` Sakari Ailus 2018-09-27 17:04 ` Matthias Kaehlcke 0 siblings, 2 replies; 13+ messages in thread From: Heikki Krogerus @ 2018-09-27 11:38 UTC (permalink / raw) To: Marcel Holtmann Cc: Matthias Kaehlcke, Greg Kroah-Hartman, Rafael J . Wysocki, Sakari Ailus, Marcin Wojtas, Andy Shevchenko Andy Shevchenko, Sinan Kaya, Johan Hedberg, Andrew Morton, Pekka Enberg, Masahiro Yamada, Alexey Dobriyan, linux-kernel, linux-bluetooth, Balakrishna Godavarthi, Loic Poulain, Brian Norris Hi Marcel, On Thu, Sep 27, 2018 at 12:24:33PM +0200, Marcel Holtmann wrote: > > Let's not fill property.c with framework specific helper functions any > > more! > > > > Those functions are completely bluetooth specific, so they do not > > belong here. The fact that some other framework already managed to > > slip their helpers in does not justify others to do the same. > > so? The firmware guys decided to put MAC addresses and BD addresses into the > firmware. So you have to deal with that. I think you have misunderstood the point. > Moving this into the Bluetooth subsystem is as pointless. I rather keep the > accessor function to firmware specific data in one place and not spread around > the whole tree. Especially once this is also provided via ACPI or some other > means. I assumed that is what the whole device_get part was suppose to > abstract. Unified device property API defines a _generic_ API that can be used by any type of device to access the device properties regardless of the way the hardware is described. Any device can use device_property_read_u8/u16/u32/u64/string() functions, but only bluetooth devices can use device_get_bd_address(). Therefore that function does not belong to drivers/base/properties.c. Thanks, -- heikki ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3.1 1/2] device property: Add device_get_bd_address() and fwnode_get_bd_address() 2018-09-27 11:38 ` Heikki Krogerus @ 2018-09-27 13:15 ` Sakari Ailus 2018-09-27 17:04 ` Matthias Kaehlcke 1 sibling, 0 replies; 13+ messages in thread From: Sakari Ailus @ 2018-09-27 13:15 UTC (permalink / raw) To: Heikki Krogerus Cc: Marcel Holtmann, Matthias Kaehlcke, Greg Kroah-Hartman, Rafael J . Wysocki, Marcin Wojtas, Andy Shevchenko Andy Shevchenko, Sinan Kaya, Johan Hedberg, Andrew Morton, Pekka Enberg, Masahiro Yamada, Alexey Dobriyan, linux-kernel, linux-bluetooth, Balakrishna Godavarthi, Loic Poulain, Brian Norris Hi guys, On Thu, Sep 27, 2018 at 02:38:29PM +0300, Heikki Krogerus wrote: > Hi Marcel, > > On Thu, Sep 27, 2018 at 12:24:33PM +0200, Marcel Holtmann wrote: > > > Let's not fill property.c with framework specific helper functions any > > > more! > > > > > > Those functions are completely bluetooth specific, so they do not > > > belong here. The fact that some other framework already managed to > > > slip their helpers in does not justify others to do the same. > > > > so? The firmware guys decided to put MAC addresses and BD addresses into the > > firmware. So you have to deal with that. > > I think you have misunderstood the point. > > > Moving this into the Bluetooth subsystem is as pointless. I rather keep the > > accessor function to firmware specific data in one place and not spread around > > the whole tree. Especially once this is also provided via ACPI or some other > > means. I assumed that is what the whole device_get part was suppose to > > abstract. > > Unified device property API defines a _generic_ API that can be used > by any type of device to access the device properties regardless of > the way the hardware is described. > > Any device can use device_property_read_u8/u16/u32/u64/string() > functions, but only bluetooth devices can use device_get_bd_address(). > Therefore that function does not belong to drivers/base/properties.c. FWIW, what's been relevant for V4L2 devices has always been parsed within the V4L2 framework. It's way more than needed by BT here though; see: drivers/media/v4l2-core/v4l2-fwnode.c I wouldn't think of putting this under drivers/base. -- Regards, Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3.1 1/2] device property: Add device_get_bd_address() and fwnode_get_bd_address() 2018-09-27 11:38 ` Heikki Krogerus 2018-09-27 13:15 ` Sakari Ailus @ 2018-09-27 17:04 ` Matthias Kaehlcke 1 sibling, 0 replies; 13+ messages in thread From: Matthias Kaehlcke @ 2018-09-27 17:04 UTC (permalink / raw) To: Heikki Krogerus Cc: Marcel Holtmann, Greg Kroah-Hartman, Rafael J . Wysocki, Sakari Ailus, Marcin Wojtas, Andy Shevchenko Andy Shevchenko, Sinan Kaya, Johan Hedberg, Andrew Morton, Pekka Enberg, Masahiro Yamada, Alexey Dobriyan, linux-kernel, linux-bluetooth, Balakrishna Godavarthi, Loic Poulain, Brian Norris On Thu, Sep 27, 2018 at 02:38:29PM +0300, Heikki Krogerus wrote: > Hi Marcel, > > On Thu, Sep 27, 2018 at 12:24:33PM +0200, Marcel Holtmann wrote: > > > Let's not fill property.c with framework specific helper functions any > > > more! > > > > > > Those functions are completely bluetooth specific, so they do not > > > belong here. The fact that some other framework already managed to > > > slip their helpers in does not justify others to do the same. > > > > so? The firmware guys decided to put MAC addresses and BD addresses into the > > firmware. So you have to deal with that. > > I think you have misunderstood the point. > > > Moving this into the Bluetooth subsystem is as pointless. I rather keep the > > accessor function to firmware specific data in one place and not spread around > > the whole tree. Especially once this is also provided via ACPI or some other > > means. I assumed that is what the whole device_get part was suppose to > > abstract. > > Unified device property API defines a _generic_ API that can be used > by any type of device to access the device properties regardless of > the way the hardware is described. > > Any device can use device_property_read_u8/u16/u32/u64/string() > functions, but only bluetooth devices can use device_get_bd_address(). > Therefore that function does not belong to drivers/base/properties.c. Initially property.[ch] seemed the correct place to me since the device_get_bd_address() is the equivalent to device_get_mac_address(), which lives there. However I doubted to use bdaddr_t in the interface when I noticed that the other functions only use generic data types. Not a red flag but a first hint that it's probably not the right place for Bluetooth specific functions. I agree with Heikki that the Bluetooth subsystem seems a better home for this API. Cheers Matthias ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3.1 2/2] Bluetooth: btqcomsmd: Get the BD address with device_get_bd_address() 2018-09-25 19:10 [PATCH v3.1 0/2] Add API to retrieve the Bluetooth Device Address (BD_ADDR) Matthias Kaehlcke 2018-09-25 19:10 ` [PATCH v3.1 1/2] device property: Add device_get_bd_address() and fwnode_get_bd_address() Matthias Kaehlcke @ 2018-09-25 19:10 ` Matthias Kaehlcke 2018-09-25 21:34 ` Sakari Ailus 1 sibling, 1 reply; 13+ messages in thread From: Matthias Kaehlcke @ 2018-09-25 19:10 UTC (permalink / raw) To: Greg Kroah-Hartman, Rafael J . Wysocki, Sakari Ailus, Marcin Wojtas, Andy Shevchenko Andy Shevchenko, Sinan Kaya, Marcel Holtmann, Johan Hedberg, Andrew Morton, Pekka Enberg, Masahiro Yamada, Alexey Dobriyan Cc: linux-kernel, linux-bluetooth, Balakrishna Godavarthi, Loic Poulain, Brian Norris, 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> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> --- Changes in v3: - added 'Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>' tag Changes in v2: - pass bdaddr_t instead of byte pointer + len --- drivers/bluetooth/btqcomsmd.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/bluetooth/btqcomsmd.c b/drivers/bluetooth/btqcomsmd.c index 7df3eed1ef5e..ff74d2c46991 100644 --- a/drivers/bluetooth/btqcomsmd.c +++ b/drivers/bluetooth/btqcomsmd.c @@ -172,11 +172,9 @@ 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, &btq->bdaddr)) dev_info(&pdev->dev, "BD address %pMR retrieved from device-tree", &btq->bdaddr); - } hdev = hci_alloc_dev(); if (!hdev) -- 2.19.0.605.g01d371f741-goog ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3.1 2/2] Bluetooth: btqcomsmd: Get the BD address with device_get_bd_address() 2018-09-25 19:10 ` [PATCH v3.1 2/2] Bluetooth: btqcomsmd: Get the BD address with device_get_bd_address() Matthias Kaehlcke @ 2018-09-25 21:34 ` Sakari Ailus 0 siblings, 0 replies; 13+ messages in thread From: Sakari Ailus @ 2018-09-25 21:34 UTC (permalink / raw) To: Matthias Kaehlcke Cc: Greg Kroah-Hartman, Rafael J . Wysocki, Marcin Wojtas, Andy Shevchenko Andy Shevchenko, Sinan Kaya, Marcel Holtmann, Johan Hedberg, Andrew Morton, Pekka Enberg, Masahiro Yamada, Alexey Dobriyan, linux-kernel, linux-bluetooth, Balakrishna Godavarthi, Loic Poulain, Brian Norris On Tue, Sep 25, 2018 at 12:10:14PM -0700, Matthias Kaehlcke wrote: > 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> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com> -- Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-09-27 17:04 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-25 19:10 [PATCH v3.1 0/2] Add API to retrieve the Bluetooth Device Address (BD_ADDR) Matthias Kaehlcke 2018-09-25 19:10 ` [PATCH v3.1 1/2] device property: Add device_get_bd_address() and fwnode_get_bd_address() Matthias Kaehlcke 2018-09-25 21:33 ` Sakari Ailus 2018-09-25 23:50 ` Matthias Kaehlcke 2018-09-26 11:36 ` Heikki Krogerus 2018-09-26 17:12 ` Andy Shevchenko 2018-09-26 21:03 ` Matthias Kaehlcke 2018-09-27 10:24 ` Marcel Holtmann 2018-09-27 11:38 ` Heikki Krogerus 2018-09-27 13:15 ` Sakari Ailus 2018-09-27 17:04 ` Matthias Kaehlcke 2018-09-25 19:10 ` [PATCH v3.1 2/2] Bluetooth: btqcomsmd: Get the BD address with device_get_bd_address() Matthias Kaehlcke 2018-09-25 21:34 ` Sakari Ailus
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).