linux-kernel - Re: [PATCH v3.1 1/2] device property: Add device_get_bd_address() and fwnode_get_bd

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 27 Sep 2018 12:24:33 +0200
From:   Marcel Holtmann <marcel@...tmann.org>
To:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Cc:     Matthias Kaehlcke <mka@...omium.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J . Wysocki" <rafael@...nel.org>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Marcin Wojtas <mw@...ihalf.com>,
        Andy Shevchenko Andy Shevchenko <andy.shevchenko@...il.com>,
        Sinan Kaya <okaya@...eaurora.org>,
        Johan Hedberg <johan.hedberg@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Pekka Enberg <penberg@...nel.org>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Alexey Dobriyan <adobriyan@...il.com>,
        linux-kernel@...r.kernel.org, linux-bluetooth@...r.kernel.org,
        Balakrishna Godavarthi <bgodavar@...eaurora.org>,
        Loic Poulain <loic.poulain@...aro.org>,
        Brian Norris <briannorris@...omium.org>
Subject: Re: [PATCH v3.1 1/2] device property: Add device_get_bd_address() and
 fwnode_get_bd_address()

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