All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
To: Bjorn Andersson
	<bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Loic Poulain
	<loic.poulain-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Johan Hedberg
	<johan.hedberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Bluez mailing list
	<linux-bluetooth-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH v6 1/3] dt-bindings: net: document Bluetooth bindings in one place
Date: Thu, 7 Sep 2017 08:14:32 +0200	[thread overview]
Message-ID: <41A0C162-4AC5-4969-813D-9E2C7F5D5031@holtmann.org> (raw)
In-Reply-To: <20170906232610.GG1165@minitux>

Hi Bjorn,

>> In the same way as Ethernet, gather the Bluetooth related bindings in
>> one file. Introduce the bluetooth-bd-address property which can be used
>> to store the assigned BD address.
>> 
>> Signed-off-by: Loic Poulain <loic.poulain-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>> v2: Set device as unconfigured if default address detected
>>     Add warning if BD addr retrieved from DT
>> v3: if no addr retrieved from DT, unconditionally set
>>     the invalid BD addr flag.
>>     swap and set bdaddr in the platform probe
>> v4: Add dt-bindings documentation
>>     split patch in two parts (setup, dt prop)
>>     use local-bd-address name instead of local-mac-address
>> v5: remove 2/3 merged in bluetooth-next tree
>>     Add bluetooth.txt for common BT bindings
>>     expect local-bd-address in little-endian format
>> v6: use of_property_read_u8_array instead of of_get_property
>> 
>> Documentation/devicetree/bindings/net/bluetooth.txt | 5 +++++
>> 1 file changed, 5 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/net/bluetooth.txt
>> 
>> diff --git a/Documentation/devicetree/bindings/net/bluetooth.txt b/Documentation/devicetree/bindings/net/bluetooth.txt
>> new file mode 100644
>> index 0000000..94797df
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/bluetooth.txt
>> @@ -0,0 +1,5 @@
>> +The following properties are common to the Bluetooth controllers:
>> +
>> +- local-bd-address: array of 6 bytes, specifies the BD address that was
>> +  uniquely assigned to the Bluetooth device, formatted with least significant
>> +  byte first (little-endian).
> 
> Is there a good reason for making this little endian?
> 
> Sure it's only going to be read and written by computers, but for anyone
> reading the generated DT it sure looks like the values are reversed.

the Bluetooth spec has a string notation and defines how it is packed as 6 octet binary.

"On the baseband level the BD_ADDR is represented as 48 bits. On the Link Layer the public and random device address are represented as 48-bit addresses."

"On the UI level the Bluetooth address shall be represented as 12 hexadecimal characters, possibly divided into sub-parts separated by ‘:’ (e.g., ‘000C3E3A4B69’ or ‘00:0C:3E:3A:4B:69’). On the UI level any number shall have the MSB -> LSB (from left to right) ‘natural’ ordering.”

However on the wire level it is LSB -> MSB. See our ba2str function.

int str2ba(const char *str, bdaddr_t *ba)
{
	int i;
	
	for (i = 5; i >= 0; i--, str += 3)
		ba->b[i] = strtol(str, NULL, 16);

	return 0;
}

I would prefer the boot loader actually providing the BD Address in the correct byte order as the protocol expects and not yet another form. The string representation is just for reference since that is what most people have seen so far.

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Marcel Holtmann <marcel@holtmann.org>
To: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Loic Poulain <loic.poulain@linaro.org>,
	Rob Herring <robh+dt@kernel.org>,
	Johan Hedberg <johan.hedberg@gmail.com>,
	Bluez mailing list <linux-bluetooth@vger.kernel.org>,
	linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v6 1/3] dt-bindings: net: document Bluetooth bindings in one place
Date: Thu, 7 Sep 2017 08:14:32 +0200	[thread overview]
Message-ID: <41A0C162-4AC5-4969-813D-9E2C7F5D5031@holtmann.org> (raw)
In-Reply-To: <20170906232610.GG1165@minitux>

Hi Bjorn,

>> In the same way as Ethernet, gather the Bluetooth related bindings in
>> one file. Introduce the bluetooth-bd-address property which can be used
>> to store the assigned BD address.
>> 
>> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
>> ---
>> v2: Set device as unconfigured if default address detected
>>     Add warning if BD addr retrieved from DT
>> v3: if no addr retrieved from DT, unconditionally set
>>     the invalid BD addr flag.
>>     swap and set bdaddr in the platform probe
>> v4: Add dt-bindings documentation
>>     split patch in two parts (setup, dt prop)
>>     use local-bd-address name instead of local-mac-address
>> v5: remove 2/3 merged in bluetooth-next tree
>>     Add bluetooth.txt for common BT bindings
>>     expect local-bd-address in little-endian format
>> v6: use of_property_read_u8_array instead of of_get_property
>> 
>> Documentation/devicetree/bindings/net/bluetooth.txt | 5 +++++
>> 1 file changed, 5 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/net/bluetooth.txt
>> 
>> diff --git a/Documentation/devicetree/bindings/net/bluetooth.txt b/Documentation/devicetree/bindings/net/bluetooth.txt
>> new file mode 100644
>> index 0000000..94797df
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/bluetooth.txt
>> @@ -0,0 +1,5 @@
>> +The following properties are common to the Bluetooth controllers:
>> +
>> +- local-bd-address: array of 6 bytes, specifies the BD address that was
>> +  uniquely assigned to the Bluetooth device, formatted with least significant
>> +  byte first (little-endian).
> 
> Is there a good reason for making this little endian?
> 
> Sure it's only going to be read and written by computers, but for anyone
> reading the generated DT it sure looks like the values are reversed.

the Bluetooth spec has a string notation and defines how it is packed as 6 octet binary.

"On the baseband level the BD_ADDR is represented as 48 bits. On the Link Layer the public and random device address are represented as 48-bit addresses."

"On the UI level the Bluetooth address shall be represented as 12 hexadecimal characters, possibly divided into sub-parts separated by ‘:’ (e.g., ‘000C3E3A4B69’ or ‘00:0C:3E:3A:4B:69’). On the UI level any number shall have the MSB -> LSB (from left to right) ‘natural’ ordering.”

However on the wire level it is LSB -> MSB. See our ba2str function.

int str2ba(const char *str, bdaddr_t *ba)
{
	int i;
	
	for (i = 5; i >= 0; i--, str += 3)
		ba->b[i] = strtol(str, NULL, 16);

	return 0;
}

I would prefer the boot loader actually providing the BD Address in the correct byte order as the protocol expects and not yet another form. The string representation is just for reference since that is what most people have seen so far.

Regards

Marcel


  reply	other threads:[~2017-09-07  6:14 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-06 12:11 [PATCH v6 1/3] dt-bindings: net: document Bluetooth bindings in one place Loic Poulain
2017-09-06 12:11 ` [PATCH v6 2/3] dt-bindings: soc: qcom: Add local-bd-address property to WCNSS-BT Loic Poulain
2017-09-06 12:11 ` [PATCH v6 3/3] Bluetooth: btqcomsmd: retrieve BD address from DT property Loic Poulain
2017-09-06 17:24   ` Marcel Holtmann
2017-09-08  7:04     ` Loic Poulain
     [not found]       ` <CAMZdPi8gSnS702LadCL7gRKzYbEtOBUZTYUrN1b_1wiycpC1gw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-09-08 10:57         ` Marcel Holtmann
2017-09-08 10:57           ` Marcel Holtmann
     [not found] ` <1504699898-28011-1-git-send-email-loic.poulain-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2017-09-06 23:26   ` [PATCH v6 1/3] dt-bindings: net: document Bluetooth bindings in one place Bjorn Andersson
2017-09-06 23:26     ` Bjorn Andersson
2017-09-07  6:14     ` Marcel Holtmann [this message]
2017-09-07  6:14       ` Marcel Holtmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=41A0C162-4AC5-4969-813D-9E2C7F5D5031@holtmann.org \
    --to=marcel-kz+m5ild9qbg9huczpvpmw@public.gmane.org \
    --cc=bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=johan.hedberg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-bluetooth-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=loic.poulain-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.