On 19/03/2024 05:20, Marek Vasut wrote:
> CYW43439 is a Wi-Fi + Bluetooth combo device from Infineon.
> The Bluetooth part is capable of Bluetooth 5.2 BR/EDR/LE .
> This chip is present e.g. on muRata 1YN module.
>
> Extend the binding with its DT compatible using fallback
> compatible string to "brcm,bcm4329-bt" which seems to be
> the oldest compatible device. This should also prevent the
> growth of compatible string tables in drivers. The existing
> block of compatible strings is retained.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
[-- Attachment #1: Type: text/plain, Size: 2129 bytes --] This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=836295 ---Test result--- Test Summary: CheckPatch PASS 0.60 seconds GitLint PASS 0.29 seconds SubjectPrefix FAIL 0.31 seconds BuildKernel PASS 27.55 seconds CheckAllWarning PASS 30.43 seconds CheckSparse PASS 36.01 seconds CheckSmatch PASS 97.81 seconds BuildKernel32 PASS 26.87 seconds TestRunnerSetup PASS 504.25 seconds TestRunner_l2cap-tester PASS 17.87 seconds TestRunner_iso-tester FAIL 30.06 seconds TestRunner_bnep-tester PASS 4.74 seconds TestRunner_mgmt-tester FAIL 107.57 seconds TestRunner_rfcomm-tester PASS 7.29 seconds TestRunner_sco-tester PASS 14.91 seconds TestRunner_ioctl-tester PASS 7.72 seconds TestRunner_mesh-tester PASS 5.82 seconds TestRunner_smp-tester PASS 6.85 seconds TestRunner_userchan-tester PASS 4.91 seconds IncrementalBuild PASS 26.18 seconds Details ############################## Test: SubjectPrefix - FAIL Desc: Check subject contains "Bluetooth" prefix Output: "Bluetooth: " prefix is not specified in the subject ############################## Test: TestRunner_iso-tester - FAIL Desc: Run iso-tester with test-runner Output: Total: 117, Passed: 116 (99.1%), Failed: 1, Not Run: 0 Failed Test Cases ISO Connect2 Suspend - Success Failed 4.221 seconds ############################## Test: TestRunner_mgmt-tester - FAIL Desc: Run mgmt-tester with test-runner Output: Total: 492, Passed: 489 (99.4%), Failed: 1, Not Run: 2 Failed Test Cases LL Privacy - Add Device 6 (RL is full) Failed 0.187 seconds --- Regards, Linux Bluetooth
CYW43439 is a Wi-Fi + Bluetooth combo device from Infineon. The Bluetooth part is capable of Bluetooth 5.2 BR/EDR/LE . This chip is present e.g. on muRata 1YN module. Extend the binding with its DT compatible using fallback compatible string to "brcm,bcm4329-bt" which seems to be the oldest compatible device. This should also prevent the growth of compatible string tables in drivers. The existing block of compatible strings is retained. Signed-off-by: Marek Vasut <marex@denx.de> --- Cc: "David S. Miller" <davem@davemloft.net> Cc: Conor Dooley <conor+dt@kernel.org> Cc: Eric Dumazet <edumazet@google.com> Cc: Jakub Kicinski <kuba@kernel.org> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> Cc: Linus Walleij <linus.walleij@linaro.org> Cc: Luiz Augusto von Dentz <luiz.dentz@gmail.com> Cc: Marcel Holtmann <marcel@holtmann.org> Cc: Paolo Abeni <pabeni@redhat.com> Cc: Rob Herring <robh@kernel.org> Cc: devicetree@vger.kernel.org Cc: linux-bluetooth@vger.kernel.org Cc: netdev@vger.kernel.org --- V2: - Introduce fallback compatible string - Reword the second half of commit message to reflect that --- .../bindings/net/broadcom-bluetooth.yaml | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml b/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml index cc70b00c6ce57..4a1bfc2b35849 100644 --- a/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml +++ b/Documentation/devicetree/bindings/net/broadcom-bluetooth.yaml @@ -14,20 +14,25 @@ description: properties: compatible: - enum: - - brcm,bcm20702a1 - - brcm,bcm4329-bt - - brcm,bcm4330-bt - - brcm,bcm4334-bt - - brcm,bcm43430a0-bt - - brcm,bcm43430a1-bt - - brcm,bcm43438-bt - - brcm,bcm4345c5 - - brcm,bcm43540-bt - - brcm,bcm4335a0 - - brcm,bcm4349-bt - - cypress,cyw4373a0-bt - - infineon,cyw55572-bt + oneOf: + - items: + - enum: + - infineon,cyw43439-bt + - const: brcm,bcm4329-bt + - enum: + - brcm,bcm20702a1 + - brcm,bcm4329-bt + - brcm,bcm4330-bt + - brcm,bcm4334-bt + - brcm,bcm43430a0-bt + - brcm,bcm43430a1-bt + - brcm,bcm43438-bt + - brcm,bcm4345c5 + - brcm,bcm43540-bt + - brcm,bcm4335a0 + - brcm,bcm4349-bt + - cypress,cyw4373a0-bt + - infineon,cyw55572-bt shutdown-gpios: maxItems: 1 -- 2.43.0
Branch: refs/heads/master Home: https://github.com/bluez/bluez Commit: 8060d1208673826665b7297c27aa75003521b52a https://github.com/bluez/bluez/commit/8060d1208673826665b7297c27aa75003521b52a Author: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> Date: 2024-03-18 (Mon, 18 Mar 2024) Changed paths: M src/device.c Log Message: ----------- device: Fix device_is_connected checking for services being connected Change 44d3f67277f83983e1e9697eda7b9aeb40ca231d seems to have introduced quite a few bugs related to device_is_connected return true which prevents proper cleanup of connection. Fixes: https://github.com/bluez/bluez/issues/774 Fixes: https://github.com/bluez/bluez/issues/778 Fixes: https://github.com/bluez/bluez/issues/783 Fixes: https://github.com/bluez/bluez/issues/784 To unsubscribe from these emails, change your notification settings at https://github.com/bluez/bluez/settings/notifications
Hello: This patch was applied to bluetooth/bluez.git (master) by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>: On Mon, 18 Mar 2024 16:08:31 +0000 you wrote: > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > Change 44d3f67277f83983e1e9697eda7b9aeb40ca231d since to have introduced > quite a few bugs related to device_is_connected return true which > prevents proper cleanup of connection. > > Fixes: https://github.com/bluez/bluez/issues/774 > Fixes: https://github.com/bluez/bluez/issues/778 > Fixes: https://github.com/bluez/bluez/issues/783 > Fixes: https://github.com/bluez/bluez/issues/784 > > [...] Here is the summary with links: - [BlueZ,v1] device: Fix device_is_connected checking for services being connected https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=8060d1208673 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
Hi Silviu, On Mon, Mar 18, 2024 at 4:39 PM Silviu Florian Barbulescu <silviu.barbulescu@nxp.com> wrote: > > update broadcast source script to support the BIS reconfiguration Please add the resulting output to the patch description. > --- > client/scripts/broadcast-source.bt | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/client/scripts/broadcast-source.bt b/client/scripts/broadcast-source.bt > index 6da9e23e2..4fb0c3920 100644 > --- a/client/scripts/broadcast-source.bt > +++ b/client/scripts/broadcast-source.bt > @@ -5,7 +5,15 @@ a > 3 > 4 > endpoint.config /org/bluez/hci0/pac_bcast0 /local/endpoint/ep0 16_2_1 > +0 > +1 > 1 > -3 > 0x03 0x02 0x04 0x00 > -transport.acquire /org/bluez/hci0/pac_bcast0/fd0 > \ No newline at end of file > +transport.acquire /org/bluez/hci0/pac_bcast0/fd0 > +transport.release /org/bluez/hci0/pac_bcast0/fd0 > +endpoint.config /org/bluez/hci0/pac_bcast0 /local/endpoint/ep0 16_2_1 > +1 > +1 > +2 > +0x03 0x02 0x04 0x01 > +transport.acquire /org/bluez/hci0/pac_bcast0/fd0 > -- > 2.39.2 > -- Luiz Augusto von Dentz
Hi Silviu, On Mon, Mar 18, 2024 at 4:39 PM Silviu Florian Barbulescu <silviu.barbulescu@nxp.com> wrote: > > If a BIS is reconfigured, the metadata and codec capabilities are updated. > Also, the BASE is updated to reflect the update. > > --- > profiles/audio/bap.c | 76 ++++++++++++++++++++++++++++++++++++++ > profiles/audio/transport.c | 6 ++- > src/shared/bap.c | 11 +++++- > 3 files changed, 91 insertions(+), 2 deletions(-) > > diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c > index 964ba9c21..e508e03ba 100644 > --- a/profiles/audio/bap.c > +++ b/profiles/audio/bap.c > @@ -580,6 +580,11 @@ static int setup_parse_bcast_qos(struct bap_setup *setup, const char *key, > return -EINVAL; > > dbus_message_iter_get_basic(iter, &qos->bcast.big); > + } else if (!strcasecmp(key, "BIS")) { > + if (var != DBUS_TYPE_BYTE) > + return -EINVAL; > + > + dbus_message_iter_get_basic(iter, &qos->bcast.bis); > } else if (!strcasecmp(key, "Options")) { > if (var != DBUS_TYPE_BYTE) > return -EINVAL; > @@ -884,6 +889,53 @@ static void setup_free(void *data) > free(setup); > } > > +static void iterate_setups(struct bap_setup *setup) > +{ > + const struct queue_entry *entry; > + struct bap_setup *ent_setup; > + uint8_t bis_cnt = 1; > + > + for (entry = queue_get_entries(setup->ep->setups); > + entry; entry = entry->next) { > + ent_setup = entry->data; > + > + if (setup->qos.bcast.big != ent_setup->qos.bcast.big) > + continue; > + > + util_iov_free(ent_setup->base, 1); > + ent_setup->base = NULL; I do wonder why we need to store the base as part of the setup? Can't we just generate it later when it is actually about to be configured into the socket like I did? > + if (setup->qos.bcast.bis == bis_cnt) { > + bt_bap_stream_config(ent_setup->stream, &setup->qos, > + setup->caps, NULL, NULL); > + bt_bap_stream_metadata(ent_setup->stream, > + setup->metadata, NULL, NULL); > + } > + > + bis_cnt++; > + } > +} > + > +static bool verify_state(struct bap_setup *setup) > +{ > + const struct queue_entry *entry; > + struct bap_setup *ent_setup; > + > + for (entry = queue_get_entries(setup->ep->setups); > + entry; entry = entry->next) { > + ent_setup = entry->data; > + > + if (setup->qos.bcast.big != ent_setup->qos.bcast.big) > + continue; > + > + if (bt_bap_stream_get_state(ent_setup->stream) == > + BT_BAP_STREAM_STATE_STREAMING) > + return false; > + } > + > + return true; > +} > + > static DBusMessage *set_configuration(DBusConnection *conn, DBusMessage *msg, > void *data) > { > @@ -925,6 +977,30 @@ static DBusMessage *set_configuration(DBusConnection *conn, DBusMessage *msg, > util_iov_free(setup->metadata, 1); > setup->metadata = util_iov_dup( > bt_bap_pac_get_metadata(ep->rpac), 1); > + } else if (bt_bap_pac_get_type(ep->lpac) == BT_BAP_BCAST_SOURCE) { > + if (setup->qos.bcast.bis != BT_ISO_QOS_BIS_UNSET) { > + if ((setup->qos.bcast.bis > queue_length(ep->setups)) || > + (setup->qos.bcast.bis == 0)) { > + setup_free(setup); > + return btd_error_invalid_args(msg); > + } > + > + /* Verify that no BIS in the BIG is in streaming state > + */ > + if (!verify_state(setup)) { > + setup_free(setup); > + return btd_error_not_permitted(msg, > + "Broadcast Audio Stream state is invalid"); > + } I was thinking we could delay the BASE setup until later when the code actually needs it then the code for stream_get_base can actually detect what are the streams with the same BIG and generate the BASE just once. > + /* Find and update the BIS specified in > + * set_configuration command > + */ > + iterate_setups(setup); > + > + setup_free(setup); > + return g_dbus_create_reply(msg, DBUS_TYPE_INVALID); > + } > } > > setup->stream = bt_bap_stream_new(ep->data->bap, ep->lpac, ep->rpac, > diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c > index 122c3339e..a060f8c61 100644 > --- a/profiles/audio/transport.c > +++ b/profiles/audio/transport.c > @@ -1643,8 +1643,12 @@ static void bap_state_changed(struct bt_bap_stream *stream, uint8_t old_state, > bap_update_links(transport); > if (!media_endpoint_is_broadcast(transport->endpoint)) > bap_update_qos(transport); > - else if (bt_bap_stream_io_dir(stream) != BT_BAP_BCAST_SOURCE) > + else if (bt_bap_stream_io_dir(stream) != BT_BAP_BCAST_SOURCE) { > bap_update_bcast_qos(transport); > + if (old_state == BT_BAP_STREAM_STATE_QOS) > + bap_update_bcast_config(transport); > + } > + > transport_update_playing(transport, FALSE); > return; > case BT_BAP_STREAM_STATE_DISABLING: > diff --git a/src/shared/bap.c b/src/shared/bap.c > index fd99cbbca..603d6d646 100644 > --- a/src/shared/bap.c > +++ b/src/shared/bap.c > @@ -1701,7 +1701,16 @@ static unsigned int bap_bcast_config(struct bt_bap_stream *stream, > struct bt_bap_qos *qos, struct iovec *data, > bt_bap_stream_func_t func, void *user_data) > { > - stream->qos = *qos; > + if (qos) { > + stream->qos = *qos; > + stream->qos.bcast.bcode = util_iov_dup(qos->bcast.bcode, 1); > + } > + > + if (data) { > + util_iov_free(stream->cc, 1); > + stream->cc = util_iov_dup(data, 1); > + } > + > stream->lpac->ops->config(stream, stream->cc, &stream->qos, > ep_config_cb, stream->lpac->user_data); > > -- > 2.39.2 These changes to shared/bap shall be submitted in a separate patch with a proper description. -- Luiz Augusto von Dentz
Hi Silviu, On Mon, Mar 18, 2024 at 4:39 PM Silviu Florian Barbulescu <silviu.barbulescu@nxp.com> wrote: > > Add support to update the transport configuration > > --- > profiles/audio/transport.c | 21 +++++++++++++++++++++ > profiles/audio/transport.h | 1 + > 2 files changed, 22 insertions(+) > > diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c > index 159fbd575..122c3339e 100644 > --- a/profiles/audio/transport.c > +++ b/profiles/audio/transport.c > @@ -1483,6 +1483,27 @@ static void bap_update_bcast_qos(const struct media_transport *transport) > "Configuration"); > } > > +void bap_update_bcast_config(struct media_transport *transport) > +{ > + struct bap_transport *bap = transport->data; > + struct iovec *cc; > + > + cc = bt_bap_stream_get_config(bap->stream); > + > + if (((int)cc->iov_len != transport->size) || > + (memcmp(cc->iov_base, transport->configuration, > + transport->size) != 0)) { Lets just use util_iov_memcmp, just init transport->configuration into an iov. > + free(transport->configuration); > + transport->configuration = util_memdup(cc->iov_base, > + cc->iov_len); > + transport->size = cc->iov_len; > + > + g_dbus_emit_property_changed(btd_get_dbus_connection(), > + transport->path, MEDIA_TRANSPORT_INTERFACE, > + "Configuration"); > + } > +} > + > static guint transport_bap_resume(struct media_transport *transport, > struct media_owner *owner) > { > diff --git a/profiles/audio/transport.h b/profiles/audio/transport.h > index b46bc8025..6df419a67 100644 > --- a/profiles/audio/transport.h > +++ b/profiles/audio/transport.h > @@ -16,6 +16,7 @@ struct media_transport *media_transport_create(struct btd_device *device, > uint8_t *configuration, > size_t size, void *data, > void *stream); > +void bap_update_bcast_config(struct media_transport *transport); > > void media_transport_destroy(struct media_transport *transport); > const char *media_transport_get_path(struct media_transport *transport); > -- > 2.39.2 > -- Luiz Augusto von Dentz
update broadcast source script to support the BIS reconfiguration --- client/scripts/broadcast-source.bt | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/client/scripts/broadcast-source.bt b/client/scripts/broadcast-source.bt index 6da9e23e2..4fb0c3920 100644 --- a/client/scripts/broadcast-source.bt +++ b/client/scripts/broadcast-source.bt @@ -5,7 +5,15 @@ a 3 4 endpoint.config /org/bluez/hci0/pac_bcast0 /local/endpoint/ep0 16_2_1 +0 +1 1 -3 0x03 0x02 0x04 0x00 -transport.acquire /org/bluez/hci0/pac_bcast0/fd0 \ No newline at end of file +transport.acquire /org/bluez/hci0/pac_bcast0/fd0 +transport.release /org/bluez/hci0/pac_bcast0/fd0 +endpoint.config /org/bluez/hci0/pac_bcast0 /local/endpoint/ep0 16_2_1 +1 +1 +2 +0x03 0x02 0x04 0x01 +transport.acquire /org/bluez/hci0/pac_bcast0/fd0 -- 2.39.2
cmd_acquire_transport and cmd_release_transport should not call bt_shell_noninteractive_quit, this will be called on acquire_reply respectively release_reply. Only on replay the acquire\release process is finished --- client/player.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/player.c b/client/player.c index c9c6779f3..49bb52f98 100644 --- a/client/player.c +++ b/client/player.c @@ -4893,7 +4893,7 @@ static void cmd_acquire_transport(int argc, char *argv[]) transport_acquire(proxy, false); } - return bt_shell_noninteractive_quit(EXIT_SUCCESS); + return; } static void release_reply(DBusMessage *message, void *user_data) @@ -4945,7 +4945,7 @@ static void cmd_release_transport(int argc, char *argv[]) } } - return bt_shell_noninteractive_quit(EXIT_SUCCESS); + return; } static int open_file(const char *filename, int flags) -- 2.39.2
If a BIS is reconfigured, the metadata and codec capabilities are updated. Also, the BASE is updated to reflect the update. --- profiles/audio/bap.c | 76 ++++++++++++++++++++++++++++++++++++++ profiles/audio/transport.c | 6 ++- src/shared/bap.c | 11 +++++- 3 files changed, 91 insertions(+), 2 deletions(-) diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c index 964ba9c21..e508e03ba 100644 --- a/profiles/audio/bap.c +++ b/profiles/audio/bap.c @@ -580,6 +580,11 @@ static int setup_parse_bcast_qos(struct bap_setup *setup, const char *key, return -EINVAL; dbus_message_iter_get_basic(iter, &qos->bcast.big); + } else if (!strcasecmp(key, "BIS")) { + if (var != DBUS_TYPE_BYTE) + return -EINVAL; + + dbus_message_iter_get_basic(iter, &qos->bcast.bis); } else if (!strcasecmp(key, "Options")) { if (var != DBUS_TYPE_BYTE) return -EINVAL; @@ -884,6 +889,53 @@ static void setup_free(void *data) free(setup); } +static void iterate_setups(struct bap_setup *setup) +{ + const struct queue_entry *entry; + struct bap_setup *ent_setup; + uint8_t bis_cnt = 1; + + for (entry = queue_get_entries(setup->ep->setups); + entry; entry = entry->next) { + ent_setup = entry->data; + + if (setup->qos.bcast.big != ent_setup->qos.bcast.big) + continue; + + util_iov_free(ent_setup->base, 1); + ent_setup->base = NULL; + + if (setup->qos.bcast.bis == bis_cnt) { + bt_bap_stream_config(ent_setup->stream, &setup->qos, + setup->caps, NULL, NULL); + bt_bap_stream_metadata(ent_setup->stream, + setup->metadata, NULL, NULL); + } + + bis_cnt++; + } +} + +static bool verify_state(struct bap_setup *setup) +{ + const struct queue_entry *entry; + struct bap_setup *ent_setup; + + for (entry = queue_get_entries(setup->ep->setups); + entry; entry = entry->next) { + ent_setup = entry->data; + + if (setup->qos.bcast.big != ent_setup->qos.bcast.big) + continue; + + if (bt_bap_stream_get_state(ent_setup->stream) == + BT_BAP_STREAM_STATE_STREAMING) + return false; + } + + return true; +} + static DBusMessage *set_configuration(DBusConnection *conn, DBusMessage *msg, void *data) { @@ -925,6 +977,30 @@ static DBusMessage *set_configuration(DBusConnection *conn, DBusMessage *msg, util_iov_free(setup->metadata, 1); setup->metadata = util_iov_dup( bt_bap_pac_get_metadata(ep->rpac), 1); + } else if (bt_bap_pac_get_type(ep->lpac) == BT_BAP_BCAST_SOURCE) { + if (setup->qos.bcast.bis != BT_ISO_QOS_BIS_UNSET) { + if ((setup->qos.bcast.bis > queue_length(ep->setups)) || + (setup->qos.bcast.bis == 0)) { + setup_free(setup); + return btd_error_invalid_args(msg); + } + + /* Verify that no BIS in the BIG is in streaming state + */ + if (!verify_state(setup)) { + setup_free(setup); + return btd_error_not_permitted(msg, + "Broadcast Audio Stream state is invalid"); + } + + /* Find and update the BIS specified in + * set_configuration command + */ + iterate_setups(setup); + + setup_free(setup); + return g_dbus_create_reply(msg, DBUS_TYPE_INVALID); + } } setup->stream = bt_bap_stream_new(ep->data->bap, ep->lpac, ep->rpac, diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c index 122c3339e..a060f8c61 100644 --- a/profiles/audio/transport.c +++ b/profiles/audio/transport.c @@ -1643,8 +1643,12 @@ static void bap_state_changed(struct bt_bap_stream *stream, uint8_t old_state, bap_update_links(transport); if (!media_endpoint_is_broadcast(transport->endpoint)) bap_update_qos(transport); - else if (bt_bap_stream_io_dir(stream) != BT_BAP_BCAST_SOURCE) + else if (bt_bap_stream_io_dir(stream) != BT_BAP_BCAST_SOURCE) { bap_update_bcast_qos(transport); + if (old_state == BT_BAP_STREAM_STATE_QOS) + bap_update_bcast_config(transport); + } + transport_update_playing(transport, FALSE); return; case BT_BAP_STREAM_STATE_DISABLING: diff --git a/src/shared/bap.c b/src/shared/bap.c index fd99cbbca..603d6d646 100644 --- a/src/shared/bap.c +++ b/src/shared/bap.c @@ -1701,7 +1701,16 @@ static unsigned int bap_bcast_config(struct bt_bap_stream *stream, struct bt_bap_qos *qos, struct iovec *data, bt_bap_stream_func_t func, void *user_data) { - stream->qos = *qos; + if (qos) { + stream->qos = *qos; + stream->qos.bcast.bcode = util_iov_dup(qos->bcast.bcode, 1); + } + + if (data) { + util_iov_free(stream->cc, 1); + stream->cc = util_iov_dup(data, 1); + } + stream->lpac->ops->config(stream, stream->cc, &stream->qos, ep_config_cb, stream->lpac->user_data); -- 2.39.2
Add support to update the transport configuration --- profiles/audio/transport.c | 21 +++++++++++++++++++++ profiles/audio/transport.h | 1 + 2 files changed, 22 insertions(+) diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c index 159fbd575..122c3339e 100644 --- a/profiles/audio/transport.c +++ b/profiles/audio/transport.c @@ -1483,6 +1483,27 @@ static void bap_update_bcast_qos(const struct media_transport *transport) "Configuration"); } +void bap_update_bcast_config(struct media_transport *transport) +{ + struct bap_transport *bap = transport->data; + struct iovec *cc; + + cc = bt_bap_stream_get_config(bap->stream); + + if (((int)cc->iov_len != transport->size) || + (memcmp(cc->iov_base, transport->configuration, + transport->size) != 0)) { + free(transport->configuration); + transport->configuration = util_memdup(cc->iov_base, + cc->iov_len); + transport->size = cc->iov_len; + + g_dbus_emit_property_changed(btd_get_dbus_connection(), + transport->path, MEDIA_TRANSPORT_INTERFACE, + "Configuration"); + } +} + static guint transport_bap_resume(struct media_transport *transport, struct media_owner *owner) { diff --git a/profiles/audio/transport.h b/profiles/audio/transport.h index b46bc8025..6df419a67 100644 --- a/profiles/audio/transport.h +++ b/profiles/audio/transport.h @@ -16,6 +16,7 @@ struct media_transport *media_transport_create(struct btd_device *device, uint8_t *configuration, size_t size, void *data, void *stream); +void bap_update_bcast_config(struct media_transport *transport); void media_transport_destroy(struct media_transport *transport); const char *media_transport_get_path(struct media_transport *transport); -- 2.39.2
endpoint.config /org/bluez/hci0/pac_bcast0 /local/endpoint/ep0 48_4_1 [/local/endpoint/ep0] BIS Index for reconfiguration? (value(1-31)/no): n [/local/endpoint/ep0] BIG (auto/value): 0 [/local/endpoint/ep0] Enter channel location (value/no): 1 [/local/endpoint/ep0] Enter Metadata (value/no): n --- client/player.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/client/player.c b/client/player.c index 8081ddc13..c9c6779f3 100644 --- a/client/player.c +++ b/client/player.c @@ -3659,6 +3659,33 @@ static void config_endpoint_iso_group(const char *input, void *user_data) } } +static void endpoint_is_reconfigure_cfg(const char *input, void *user_data) +{ + struct endpoint_config *cfg = user_data; + int value; + char *endptr = NULL; + + if (!strcasecmp(input, "n") || !strcasecmp(input, "no")) { + cfg->ep->iso_stream = BT_ISO_QOS_STREAM_UNSET; + } else { + value = strtol(input, &endptr, 0); + + if (!endptr || *endptr != '\0' || value > UINT8_MAX) { + bt_shell_printf("Invalid argument: %s\n", input); + return bt_shell_noninteractive_quit(EXIT_FAILURE); + } + + if (value == 0x0) + cfg->ep->iso_stream = BT_ISO_QOS_STREAM_UNSET; + else + cfg->ep->iso_stream = value; + } + + bt_shell_prompt_input(cfg->ep->path, + "BIG (auto/value):", + config_endpoint_iso_group, cfg); +} + static void endpoint_set_config_bcast(struct endpoint_config *cfg) { cfg->ep->bcode = g_new0(struct iovec, 1); @@ -3674,8 +3701,8 @@ static void endpoint_set_config_bcast(struct endpoint_config *cfg) } bt_shell_prompt_input(cfg->ep->path, - "BIG (auto/value):", - config_endpoint_iso_group, cfg); + "BIS Index for reconfiguration? (value(1-31)/no):", + endpoint_is_reconfigure_cfg, cfg); } static void cmd_config_endpoint(int argc, char *argv[]) -- 2.39.2
This patch adds support for broadcast source to reconfigure a BIS. endpoint.config command has a new prompt for broadcast source: BIS Index for reconfiguration? (value(1-31)/no): Values n or 0 represent that no reconfiguration is required Values between (1-31) specify which BIS to be reconfigured example form client/scripts/broadcast-source.bt endpoint.register 00001852-0000-1000-8000-00805f9b34fb 0x06 endpoint.config /org/bluez/hci0/pac_bcast0 /local/endpoint/ep0 16_2_1 BIS Index for reconfiguration? (value(1-31)/no): 0 [/local/endpoint/ep0] BIG (auto/value): 1 [/local/endpoint/ep0] Enter channel location (value/no): 1 [/local/endpoint/ep0] Enter Metadata (value/no): 0x03 0x02 0x04 0x00 transport.acquire /org/bluez/hci0/pac_bcast0/fd0 HCI Command: LE Set Periodic Ad.. (0x08|0x003f) plen 41 Handle: 1 Operation: Complete ext advertising data (0x03) Data length: 0x26 Service Data: Basic Audio Announcement (0x1851) Presetation Delay: 40000 Number of Subgroups: 1 Subgroup #0: Number of BIS(s): 1 Codec: LC3 (0x06) Codec Specific Configuration: #0: len 0x02 type 0x01 Codec Specific Configuration: Sampling Frequency: 16 Khz (0x03) Codec Specific Configuration: #1: len 0x02 type 0x02 Codec Specific Configuration: Frame Duration: 10 ms (0x01) Codec Specific Configuration: #2: len 0x03 type 0x04 Codec Specific Configuration: Frame Length: 40 (0x0028) Codec Specific Configuration: #3: len 0x05 type 0x03 Codec Specific Configuration: Location: 0x00000001 Codec Specific Configuration: Location: Front Left (0x00000001) Metadata: #0: len 0x03 type 0x02 Metadata: Context: 0x0004 Metadata: Context Media (0x0004) BIS #0: Index: 1 transport.release /org/bluez/hci0/pac_bcast0/fd0 endpoint.config /org/bluez/hci0/pac_bcast0 /local/endpoint/ep0 16_2_1 [/local/endpoint/ep0] BIS Index for reconfiguration? (value(1-31)/no): 1 [/local/endpoint/ep0] BIG (auto/value): 1 [/local/endpoint/ep0] Enter channel location (value/no): 2 [/local/endpoint/ep0] Enter Metadata (value/no): 0x03 0x02 0x04 0x01 transport.acquire /org/bluez/hci0/pac_bcast0/fd0 HCI Command: LE Set Periodic Ad.. (0x08|0x003f) plen 41 #47 [hci0] Handle: 1 Operation: Complete ext advertising data (0x03) Data length: 0x26 Service Data: Basic Audio Announcement (0x1851) Presetation Delay: 40000 Number of Subgroups: 1 Subgroup #0: Number of BIS(s): 1 Codec: LC3 (0x06) Codec Specific Configuration: #0: len 0x02 type 0x01 Codec Specific Configuration: Sampling Frequency: 16 Khz (0x03) Codec Specific Configuration: #1: len 0x02 type 0x02 Codec Specific Configuration: Frame Duration: 10 ms (0x01) Codec Specific Configuration: #2: len 0x03 type 0x04 Codec Specific Configuration: Frame Length: 40 (0x0028) Codec Specific Configuration: #3: len 0x05 type 0x03 Codec Specific Configuration: Location: 0x00000002 Codec Specific Configuration: Location: Front Right (0x00000002) Metadata: #0: len 0x03 type 0x02 Metadata: Context: 0x0104 Metadata: Context Media (0x0004) Metadata: Context Notifications (0x0100) BIS #0: Index: 1 Silviu Florian Barbulescu (5): player: Add reconfiguration prompt for broadcast source transport: Add support to update the transport configuration bap: Broadcast source reconfiguration support added player.c: Remove bt_shell_noninteractive_quit on acquire,release commands client: update broadcast source script to support the BIS reconfiguration client/player.c | 35 ++++++++++++-- client/scripts/broadcast-source.bt | 12 ++++- profiles/audio/bap.c | 76 ++++++++++++++++++++++++++++++ profiles/audio/transport.c | 27 ++++++++++- profiles/audio/transport.h | 1 + src/shared/bap.c | 11 ++++- 6 files changed, 154 insertions(+), 8 deletions(-) base-commit: 84628e5d109cbec0bbd515c12c4b5224380784fe -- 2.39.2
On Mon, Mar 18, 2024 at 08:58:40AM -0700, Doug Anderson wrote: > On Mon, Mar 18, 2024 at 8:47 AM Johan Hovold <johan@kernel.org> wrote: > > On Mon, Mar 18, 2024 at 08:31:09AM -0700, Doug Anderson wrote: > > Thanks for the details. Sounds like we could get away with adding a new > > property for the broken firmware in this case, which should resolve this > > nicely without having to deprecate anything. > > > > Could you carry such a devicetree patch out-of-tree until the firmware > > has been fixed? > > IMO we shouldn't try to fix the firmware at all. Given the fact that > it took me a year to get a firmware uprev completed for one trogdor > variant for fixes that actually had functional impact, it's possible > we'll never actually get an uprev completed that includes this fix or > it will happen years from now when nobody remembers about it. I'm also > certain this whole issue will also cause a bunch of debugging over the > years if we try to fix it in firmware like that. There are cases where > people end up running with old firmware since the developer workflow > doesn't automatically update it. > > The handling should be added upstream and we should just accept that > the trogdor firmware gets it backward. Fair enough. Rob, are you OK with adding a 'qcom,local-bd-address-broken' or similarly named property to indicate that the boot firmware passes the address in the wrong order? I'd then add that property to sc7180-trogdor.dtsi in mainline. Johan
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> Change 44d3f67277f83983e1e9697eda7b9aeb40ca231d since to have introduced quite a few bugs related to device_is_connected return true which prevents proper cleanup of connection. Fixes: https://github.com/bluez/bluez/issues/774 Fixes: https://github.com/bluez/bluez/issues/778 Fixes: https://github.com/bluez/bluez/issues/783 Fixes: https://github.com/bluez/bluez/issues/784 --- src/device.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/device.c b/src/device.c index aecceb100..b5b574233 100644 --- a/src/device.c +++ b/src/device.c @@ -3324,22 +3324,24 @@ void device_add_connection(struct btd_device *dev, uint8_t bdaddr_type, "Connected"); } +static bool device_service_connected(struct btd_device *dev) +{ + if (find_service_with_state(dev->services, + BTD_SERVICE_STATE_CONNECTING)) + return true; + + return find_service_with_state(dev->services, + BTD_SERVICE_STATE_CONNECTED); +} + static bool device_disappeared(gpointer user_data) { struct btd_device *dev = user_data; - if (btd_device_is_connected(dev)) { - char addr[18]; - ba2str(&dev->bdaddr, addr); - DBG("Device %s is marked as connected", dev->path); - return TRUE; - } - - /* If there are services connecting restart the timer to give more time + /* If there are services connected restart the timer to give more time * for the service to either complete the connection or disconnect. */ - if (find_service_with_state(dev->services, - BTD_SERVICE_STATE_CONNECTING)) + if (device_service_connected(dev)) return TRUE; dev->temporary_timer = 0; -- 2.44.0
Hi,
On Mon, Mar 18, 2024 at 8:47 AM Johan Hovold <johan@kernel.org> wrote:
>
> On Mon, Mar 18, 2024 at 08:31:09AM -0700, Doug Anderson wrote:
> > On Mon, Mar 18, 2024 at 8:26 AM Doug Anderson <dianders@google.com> wrote:
>
> > > > A new compatible string (or one-off property) would allow them do make a
> > > > change when they are ready (e.g. by only updating the devicetrees after
> > > > all boot firmware has been patched and pushed out).
> > >
> > > I have no real opinion about the exact way this is solved so happy to
> > > let DT folks decide on how they want this. I will note, however, that
> > > device trees are never shipped separately and thus we have no
> > > intrinsic need for DT backward compatbility here. It would be OK from
> > > a ChromeOS perspective to add a property or compatible string for the
> > > broken case.
> >
> > Actually, I should probably say more about this to make it clear how it works.
> >
> > Chromebooks ship the kernel as a FIT image which bundles the kernel
> > and device trees together. The firmware looks at all the bundled
> > device trees and picks the proper one based on the board name,
> > revision, and SKU ID. The firmware then looks for the bluetooth node
> > (I believe it finds it from the "aliases" section) and adds the MAC
> > address there.
> >
> > ...so we could update the DT to add a property (if that's desired)
> > even if we don't update the firmware.
>
> Thanks for the details. Sounds like we could get away with adding a new
> property for the broken firmware in this case, which should resolve this
> nicely without having to deprecate anything.
>
> Could you carry such a devicetree patch out-of-tree until the firmware
> has been fixed?
IMO we shouldn't try to fix the firmware at all. Given the fact that
it took me a year to get a firmware uprev completed for one trogdor
variant for fixes that actually had functional impact, it's possible
we'll never actually get an uprev completed that includes this fix or
it will happen years from now when nobody remembers about it. I'm also
certain this whole issue will also cause a bunch of debugging over the
years if we try to fix it in firmware like that. There are cases where
people end up running with old firmware since the developer workflow
doesn't automatically update it.
The handling should be added upstream and we should just accept that
the trogdor firmware gets it backward.
-Doug
On Mon, Mar 18, 2024 at 08:31:09AM -0700, Doug Anderson wrote: > On Mon, Mar 18, 2024 at 8:26 AM Doug Anderson <dianders@google.com> wrote: > > > A new compatible string (or one-off property) would allow them do make a > > > change when they are ready (e.g. by only updating the devicetrees after > > > all boot firmware has been patched and pushed out). > > > > I have no real opinion about the exact way this is solved so happy to > > let DT folks decide on how they want this. I will note, however, that > > device trees are never shipped separately and thus we have no > > intrinsic need for DT backward compatbility here. It would be OK from > > a ChromeOS perspective to add a property or compatible string for the > > broken case. > > Actually, I should probably say more about this to make it clear how it works. > > Chromebooks ship the kernel as a FIT image which bundles the kernel > and device trees together. The firmware looks at all the bundled > device trees and picks the proper one based on the board name, > revision, and SKU ID. The firmware then looks for the bluetooth node > (I believe it finds it from the "aliases" section) and adds the MAC > address there. > > ...so we could update the DT to add a property (if that's desired) > even if we don't update the firmware. Thanks for the details. Sounds like we could get away with adding a new property for the broken firmware in this case, which should resolve this nicely without having to deprecate anything. Could you carry such a devicetree patch out-of-tree until the firmware has been fixed? Johan
Hi,
On Mon, Mar 18, 2024 at 8:26 AM Doug Anderson <dianders@google.com> wrote:
>
> Hi,
>
> On Mon, Mar 18, 2024 at 8:10 AM Johan Hovold <johan@kernel.org> wrote:
> >
> > > > I wanted to avoid doing this, but if we have to support Google's broken
> > > > boot firmware for these devices, then this is how it needs to be done.
> > >
> > > Don't Chromebooks update everything together. So maybe we don't care in
> > > this case?
> >
> > That was my hope, but Matthias seemed to suggest that we need to
> > continue supporting the current (broken) binding because doing such a
> > coordinated update may be easier said than done:
> >
> > https://lore.kernel.org/lkml/ZcuQ2qRX0zsLSVRL@google.com/
>
> Chromebooks update kernel and devicetree together, but not firmware.
> Firmware is relatively hard to get updated trying to have kernel and
> firmware updates coordinated at the exact same time has challenges.
> This would further be complicated by the fact that firmware
> qualification for each variant happens on its own timeline.
>
>
> > A new compatible string (or one-off property) would allow them do make a
> > change when they are ready (e.g. by only updating the devicetrees after
> > all boot firmware has been patched and pushed out).
>
> I have no real opinion about the exact way this is solved so happy to
> let DT folks decide on how they want this. I will note, however, that
> device trees are never shipped separately and thus we have no
> intrinsic need for DT backward compatbility here. It would be OK from
> a ChromeOS perspective to add a property or compatible string for the
> broken case.
Actually, I should probably say more about this to make it clear how it works.
Chromebooks ship the kernel as a FIT image which bundles the kernel
and device trees together. The firmware looks at all the bundled
device trees and picks the proper one based on the board name,
revision, and SKU ID. The firmware then looks for the bluetooth node
(I believe it finds it from the "aliases" section) and adds the MAC
address there.
...so we could update the DT to add a property (if that's desired)
even if we don't update the firmware.
-Doug
Hi, On Mon, Mar 18, 2024 at 8:10 AM Johan Hovold <johan@kernel.org> wrote: > > > > I wanted to avoid doing this, but if we have to support Google's broken > > > boot firmware for these devices, then this is how it needs to be done. > > > > Don't Chromebooks update everything together. So maybe we don't care in > > this case? > > That was my hope, but Matthias seemed to suggest that we need to > continue supporting the current (broken) binding because doing such a > coordinated update may be easier said than done: > > https://lore.kernel.org/lkml/ZcuQ2qRX0zsLSVRL@google.com/ Chromebooks update kernel and devicetree together, but not firmware. Firmware is relatively hard to get updated trying to have kernel and firmware updates coordinated at the exact same time has challenges. This would further be complicated by the fact that firmware qualification for each variant happens on its own timeline. > A new compatible string (or one-off property) would allow them do make a > change when they are ready (e.g. by only updating the devicetrees after > all boot firmware has been patched and pushed out). I have no real opinion about the exact way this is solved so happy to let DT folks decide on how they want this. I will note, however, that device trees are never shipped separately and thus we have no intrinsic need for DT backward compatbility here. It would be OK from a ChromeOS perspective to add a property or compatible string for the broken case. -Doug
On Mon, Mar 18, 2024 at 09:48:06AM -0500, Rob Herring wrote: > On Mon, Mar 18, 2024 at 02:17:47PM +0100, Johan Hovold wrote: > > On Mon, Mar 18, 2024 at 03:00:40PM +0200, Dmitry Baryshkov wrote: > > > On Mon, 18 Mar 2024 at 13:09, Johan Hovold <johan+linaro@kernel.org> wrote: > > > > The only device out there that should be affected by this is the WCN3991 > > > > used in some Chromebooks. To maintain backwards compatibility, mark the > > > > current compatible string as deprecated and add a new > > > > 'qcom,wcn3991-bt-bdaddr-le' for firmware which conforms with the > > > > binding. > > > > > This compatible doesn't describe new hardware kind. As such, I think, > > > the better way would be to continue using qcom,wcn3991-bt compatible > > > string + add some kind of qcom,bt-addr-le property. > > > > No, you can't handle backwards compatibility by *adding* a property. > > But you could add a property for the not broken case. That's a bit odd, > but so is your compatible. Sure, we could have a property that we only add for wcn3991-bt going forward. But we can't go back in time and add this property to all devicetrees and say that 'local-bd-address' is big endian unless that property is present (as that would leave all the non-wcn3991 devicetrees broken). > > I wanted to avoid doing this, but if we have to support Google's broken > > boot firmware for these devices, then this is how it needs to be done. > > Don't Chromebooks update everything together. So maybe we don't care in > this case? That was my hope, but Matthias seemed to suggest that we need to continue supporting the current (broken) binding because doing such a coordinated update may be easier said than done: https://lore.kernel.org/lkml/ZcuQ2qRX0zsLSVRL@google.com/ A new compatible string (or one-off property) would allow them do make a change when they are ready (e.g. by only updating the devicetrees after all boot firmware has been patched and pushed out). Johan
On Mon, Mar 18, 2024 at 02:17:47PM +0100, Johan Hovold wrote: > On Mon, Mar 18, 2024 at 03:00:40PM +0200, Dmitry Baryshkov wrote: > > On Mon, 18 Mar 2024 at 13:09, Johan Hovold <johan+linaro@kernel.org> wrote: > > > > > > Several Qualcomm Bluetooth controllers lack persistent storage for the > > > device address and instead one can be provided by the boot firmware > > > using the 'local-bd-address' devicetree property. > > > > > > The Bluetooth bindings clearly says that the address should be specified > > > in little-endian order, but due to a long-standing bug in the Qualcomm > > > driver which reversed the address some bootloaders have been providing > > > the address in big-endian order instead. > > > > > > The only device out there that should be affected by this is the WCN3991 > > > used in some Chromebooks. To maintain backwards compatibility, mark the > > > current compatible string as deprecated and add a new > > > 'qcom,wcn3991-bt-bdaddr-le' for firmware which conforms with the > > > binding. > > > This compatible doesn't describe new hardware kind. As such, I think, > > the better way would be to continue using qcom,wcn3991-bt compatible > > string + add some kind of qcom,bt-addr-le property. > > No, you can't handle backwards compatibility by *adding* a property. But you could add a property for the not broken case. That's a bit odd, but so is your compatible. > I wanted to avoid doing this, but if we have to support Google's broken > boot firmware for these devices, then this is how it needs to be done. Don't Chromebooks update everything together. So maybe we don't care in this case? Rob
On Mon, Mar 18, 2024 at 04:17:24PM +0200, Dmitry Baryshkov wrote:
> On Mon, 18 Mar 2024 at 15:17, Johan Hovold <johan@kernel.org> wrote:
> > On Mon, Mar 18, 2024 at 03:00:40PM +0200, Dmitry Baryshkov wrote:
> > > On Mon, 18 Mar 2024 at 13:09, Johan Hovold <johan+linaro@kernel.org> wrote:
> > > > The only device out there that should be affected by this is the WCN3991
> > > > used in some Chromebooks. To maintain backwards compatibility, mark the
> > > > current compatible string as deprecated and add a new
> > > > 'qcom,wcn3991-bt-bdaddr-le' for firmware which conforms with the
> > > > binding.
> >
> > > This compatible doesn't describe new hardware kind. As such, I think,
> > > the better way would be to continue using qcom,wcn3991-bt compatible
> > > string + add some kind of qcom,bt-addr-le property.
> >
> > No, you can't handle backwards compatibility by *adding* a property.
> >
> > I wanted to avoid doing this, but if we have to support Google's broken
> > boot firmware for these devices, then this is how it needs to be done.
>
> One hardware compat string per hardware type.
Again, no. Not when there is an incompatible change in the binding. Then
we add a new compatible string and deprecate the old binding.
Johan
On Mon, 18 Mar 2024 at 15:17, Johan Hovold <johan@kernel.org> wrote:
>
> On Mon, Mar 18, 2024 at 03:00:40PM +0200, Dmitry Baryshkov wrote:
> > On Mon, 18 Mar 2024 at 13:09, Johan Hovold <johan+linaro@kernel.org> wrote:
> > >
> > > Several Qualcomm Bluetooth controllers lack persistent storage for the
> > > device address and instead one can be provided by the boot firmware
> > > using the 'local-bd-address' devicetree property.
> > >
> > > The Bluetooth bindings clearly says that the address should be specified
> > > in little-endian order, but due to a long-standing bug in the Qualcomm
> > > driver which reversed the address some bootloaders have been providing
> > > the address in big-endian order instead.
> > >
> > > The only device out there that should be affected by this is the WCN3991
> > > used in some Chromebooks. To maintain backwards compatibility, mark the
> > > current compatible string as deprecated and add a new
> > > 'qcom,wcn3991-bt-bdaddr-le' for firmware which conforms with the
> > > binding.
>
> > This compatible doesn't describe new hardware kind. As such, I think,
> > the better way would be to continue using qcom,wcn3991-bt compatible
> > string + add some kind of qcom,bt-addr-le property.
>
> No, you can't handle backwards compatibility by *adding* a property.
>
> I wanted to avoid doing this, but if we have to support Google's broken
> boot firmware for these devices, then this is how it needs to be done.
One hardware compat string per hardware type.
--
With best wishes
Dmitry
On Mon, Mar 18, 2024 at 03:00:40PM +0200, Dmitry Baryshkov wrote: > On Mon, 18 Mar 2024 at 13:09, Johan Hovold <johan+linaro@kernel.org> wrote: > > > > Several Qualcomm Bluetooth controllers lack persistent storage for the > > device address and instead one can be provided by the boot firmware > > using the 'local-bd-address' devicetree property. > > > > The Bluetooth bindings clearly says that the address should be specified > > in little-endian order, but due to a long-standing bug in the Qualcomm > > driver which reversed the address some bootloaders have been providing > > the address in big-endian order instead. > > > > The only device out there that should be affected by this is the WCN3991 > > used in some Chromebooks. To maintain backwards compatibility, mark the > > current compatible string as deprecated and add a new > > 'qcom,wcn3991-bt-bdaddr-le' for firmware which conforms with the > > binding. > This compatible doesn't describe new hardware kind. As such, I think, > the better way would be to continue using qcom,wcn3991-bt compatible > string + add some kind of qcom,bt-addr-le property. No, you can't handle backwards compatibility by *adding* a property. I wanted to avoid doing this, but if we have to support Google's broken boot firmware for these devices, then this is how it needs to be done. Johan