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

[Marcel Holtmann <marcel@holtmann.org>]

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