Message ID | 20231227101003.10534-1-johan+linaro@kernel.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-11948-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7301:6f82:b0:100:9c79:88ff with SMTP id tb2csp1343950dyb; Wed, 27 Dec 2023 02:11:08 -0800 (PST) X-Google-Smtp-Source: AGHT+IFmeSYLIFNxVlaK/T6RFvUkfSYPtXs6HakpuyA9J/7u3oaVnZtcnySkNIT1iaXBWBiKPiGT X-Received: by 2002:a05:6359:c1f:b0:172:b39d:2ae4 with SMTP id gn31-20020a0563590c1f00b00172b39d2ae4mr5037654rwb.19.1703671868446; Wed, 27 Dec 2023 02:11:08 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1703671868; cv=none; d=google.com; s=arc-20160816; b=Z01qZ59tYtk7nnMztsFhJcHKKammBJYhu4Y9z5//zc0N3jlkn57jzZGl/dXtwDFW9Z Of5iOLI303y9oPfrGpDINiPQQ3iFODUdmXzjOWOR+KbDqBUefuyygqW4xL5Piuj3t9a0 JZbDyG0FvltPWYi8JLEnue3UFVGlHJRWJXcH2yFGeLeZDoi/QToPwkOPB4x3rGziioi9 1WS3OLOQ2bKwkkM5ZJN3V0xUKstkp3uKGfhSNgJXqKGJ5R+8v+HCW1f0gGIJttH6PynA AR6yrav+QP7Iu/fYVfybOjivxLsThEXOfxESPuhx6+NOM0JwmWMQN2JIiD5/MLY1KGf1 gwfg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:message-id:date:subject:cc:to :from:dkim-signature; bh=FXV/9CTRwRGx6Id56quWsJE8ZuocQj6UkA3IPfNWKu8=; fh=HvX4MXumK0mWi/uecPr6fGFwCMBxb7Yw5+pClJW60y0=; b=V0QMOUkceNXRTwPHfZyRLO0LgIi4/U3NvzXfYM93ruiv2HYWzsBAQLKmFO9cDwr7fJ nC3eLG5Rp9pAtPM2wsZTGElQ29eJUH9P4bMqk+mBUpczTNAeYHG4rg2in64dfq0Xh94E 13V1+FMq8ulzdLTvBeOEpvWtA0Hbd9Q0IwigL3z34HPUvbaZOL/ux03M21X0zPYjAVb8 0bRVmxIEJaH5bRU/NQHCiZkBIwbuKHR52/p5mebMFNSPHQK+InK0lujih2nf+lVYpfS1 LZmfg5Jfx3bwfgVOHHEDd3CVn5haXk6jwobbeoKEsgKayk3v8Rs3xzwr/MoZ7hcl+Mi+ MyPQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=OE5gY3Rg; spf=pass (google.com: domain of linux-kernel+bounces-11948-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-11948-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org. [2604:1380:45e3:2400::1]) by mx.google.com with ESMTPS id t4-20020a17090aba8400b0028bca7b7a64si10986030pjr.171.2023.12.27.02.11.08 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 27 Dec 2023 02:11:08 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-11948-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=OE5gY3Rg; spf=pass (google.com: domain of linux-kernel+bounces-11948-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-11948-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sv.mirrors.kernel.org (Postfix) with ESMTPS id B5E83282BE8 for <ouuuleilei@gmail.com>; Wed, 27 Dec 2023 10:11:06 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 2EF652D61D; Wed, 27 Dec 2023 10:10:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="OE5gY3Rg" X-Original-To: linux-kernel@vger.kernel.org Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 731F42D602; Wed, 27 Dec 2023 10:10:50 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E1F71C433C8; Wed, 27 Dec 2023 10:10:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1703671849; bh=HNX2qe5wIUjgAOE0bih0cYXbYbw77i5qRYAGTFhWf8Q=; h=From:To:Cc:Subject:Date:From; b=OE5gY3RgSNMMNpr/DemitpLvv8B5KUxkea+huKa97KhEilNAbl/OG/gZPSswoI3kl ifwN2LiGgFt8jyQh8hVsCoIrrOOGgiqeJ9ivyZI86fxAmaICglCBVc65vzpPozQdYn YsryqO/D6A6i3egHRr7G52DifE/DihYN9uZqhAIHTY+ESsr/mJvwodvV8HNt60TaF9 dFeWSj4rAgt/QaclFNs1wznkamzzJ3UloOq2NJbzC3F6ILEsWfuPRm8iKBlgwdNTZ+ bn4JmN3Zh0Mr2uaTVg902uWeLcZ93sY9BmqH1AOBDEXTlUvPd7n6xVrSnOLfdlTR5k O5JKLTyyjvyTQ== Received: from johan by xi.lan with local (Exim 4.96.2) (envelope-from <johan+linaro@kernel.org>) id 1rIQs7-0002kc-0u; Wed, 27 Dec 2023 11:10:44 +0100 From: Johan Hovold <johan+linaro@kernel.org> To: Luiz Augusto von Dentz <luiz.dentz@gmail.com>, Marcel Holtmann <marcel@holtmann.org>, Johan Hedberg <johan.hedberg@gmail.com> Cc: Hector Martin <marcan@marcan.st>, Sven Peter <sven@svenpeter.dev>, Alyssa Rosenzweig <alyssa@rosenzweig.io>, asahi@lists.linux.dev, linux-arm-kernel@lists.infradead.org, linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org, Johan Hovold <johan+linaro@kernel.org>, stable@vger.kernel.org, Felix Zhang <mrman@mrman314.tech> Subject: [PATCH] Bluetooth: hci_bcm4377: do not mark valid bd_addr as invalid Date: Wed, 27 Dec 2023 11:10:03 +0100 Message-ID: <20231227101003.10534-1-johan+linaro@kernel.org> X-Mailer: git-send-email 2.41.0 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1786429433024977126 X-GMAIL-MSGID: 1786429433024977126 |
Series |
Bluetooth: hci_bcm4377: do not mark valid bd_addr as invalid
|
|
Commit Message
Johan Hovold
Dec. 27, 2023, 10:10 a.m. UTC
A recent commit restored the original (and still documented) semantics
for the HCI_QUIRK_USE_BDADDR_PROPERTY quirk so that the device address
is considered invalid unless an address is provided by firmware.
This specifically means that this flag must only be set for devices with
invalid addresses, but the Broadcom BCM4377 driver has so far been
setting this flag unconditionally.
Fortunately the driver already checks for invalid addresses during setup
and sets the HCI_QUIRK_INVALID_BDADDR flag, which can simply be replaced
with HCI_QUIRK_USE_BDADDR_PROPERTY to indicate that the default address
is invalid but can be overridden by firmware (long term, this should
probably just always be allowed).
Fixes: 6945795bc81a ("Bluetooth: fix use-bdaddr-property quirk")
Cc: stable@vger.kernel.org # 6.5
Reported-by: Felix Zhang <mrman@mrman314.tech>
Link: https://lore.kernel.org/r/77419ffacc5b4875e920e038332575a2a5bff29f.camel@mrman314.tech/
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
drivers/bluetooth/hci_bcm4377.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Comments
On Wed, 2023-12-27 at 11:10 +0100, Johan Hovold wrote: > A recent commit restored the original (and still documented) > semantics > for the HCI_QUIRK_USE_BDADDR_PROPERTY quirk so that the device > address > is considered invalid unless an address is provided by firmware. > > This specifically means that this flag must only be set for devices > with > invalid addresses, but the Broadcom BCM4377 driver has so far been > setting this flag unconditionally. > > Fortunately the driver already checks for invalid addresses during > setup > and sets the HCI_QUIRK_INVALID_BDADDR flag, which can simply be > replaced > with HCI_QUIRK_USE_BDADDR_PROPERTY to indicate that the default > address > is invalid but can be overridden by firmware (long term, this should > probably just always be allowed). > > Fixes: 6945795bc81a ("Bluetooth: fix use-bdaddr-property quirk") > Cc: stable@vger.kernel.org # 6.5 > Reported-by: Felix Zhang <mrman@mrman314.tech> > Link: > https://lore.kernel.org/r/77419ffacc5b4875e920e038332575a2a5bff29f.camel@mrman314.tech/ > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > --- > drivers/bluetooth/hci_bcm4377.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/bluetooth/hci_bcm4377.c > b/drivers/bluetooth/hci_bcm4377.c > index a61757835695..9a7243d5db71 100644 > --- a/drivers/bluetooth/hci_bcm4377.c > +++ b/drivers/bluetooth/hci_bcm4377.c > @@ -1417,7 +1417,7 @@ static int bcm4377_check_bdaddr(struct > bcm4377_data *bcm4377) > > bda = (struct hci_rp_read_bd_addr *)skb->data; > if (!bcm4377_is_valid_bdaddr(bcm4377, &bda->bdaddr)) > - set_bit(HCI_QUIRK_INVALID_BDADDR, &bcm4377->hdev- > >quirks); > + set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &bcm4377- > >hdev->quirks); > > kfree_skb(skb); > return 0; > @@ -2368,7 +2368,6 @@ static int bcm4377_probe(struct pci_dev *pdev, > const struct pci_device_id *id) > hdev->set_bdaddr = bcm4377_hci_set_bdaddr; > hdev->setup = bcm4377_hci_setup; > > - set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks); > if (bcm4377->hw->broken_mws_transport_config) > set_bit(HCI_QUIRK_BROKEN_MWS_TRANSPORT_CONFIG, > &hdev->quirks); > if (bcm4377->hw->broken_ext_scan) Hi, Thanks for the patch! I have just tested the patch on my MacBookAir9,1 with some modifications to make it patch with the master branch of the Linux kernel, and it works great! The device is recognized by the system; and I can scan, pair, connect, and play audio just fine. If you want me to send over the modified patch that I have used, please let me know. Thanks!
On Wed, Dec 27, 2023 at 01:19:39PM -0500, Felix Zhang wrote: > On Wed, 2023-12-27 at 11:10 +0100, Johan Hovold wrote: > > A recent commit restored the original (and still documented) > > semantics > > for the HCI_QUIRK_USE_BDADDR_PROPERTY quirk so that the device > > address > > is considered invalid unless an address is provided by firmware. > > > > This specifically means that this flag must only be set for devices > > with > > invalid addresses, but the Broadcom BCM4377 driver has so far been > > setting this flag unconditionally. > > > > Fortunately the driver already checks for invalid addresses during > > setup > > and sets the HCI_QUIRK_INVALID_BDADDR flag, which can simply be > > replaced > > with HCI_QUIRK_USE_BDADDR_PROPERTY to indicate that the default > > address > > is invalid but can be overridden by firmware (long term, this should > > probably just always be allowed). > > > > Fixes: 6945795bc81a ("Bluetooth: fix use-bdaddr-property quirk") > > Cc: stable@vger.kernel.org # 6.5 > > Reported-by: Felix Zhang <mrman@mrman314.tech> > > Link: > > https://lore.kernel.org/r/77419ffacc5b4875e920e038332575a2a5bff29f.camel@mrman314.tech/ > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > Thanks for the patch! I have just tested the patch on my MacBookAir9,1 > with some modifications to make it patch with the master branch of the > Linux kernel, and it works great! The device is recognized by the > system; and I can scan, pair, connect, and play audio just fine. If > you want me to send over the modified patch that I have used, please > let me know. Thanks for confirming. You should not need to modify the patch, though, as it has been created against 6.7-rc6 and there hasn't been any changes to this driver since rc1. Johan
On Wed, Dec 27, 2023 at 5:10 AM Johan Hovold <johan+linaro@kernel.org> wrote: > > A recent commit restored the original (and still documented) semantics > for the HCI_QUIRK_USE_BDADDR_PROPERTY quirk so that the device address > is considered invalid unless an address is provided by firmware. > > This specifically means that this flag must only be set for devices with > invalid addresses, but the Broadcom BCM4377 driver has so far been > setting this flag unconditionally. > > Fortunately the driver already checks for invalid addresses during setup > and sets the HCI_QUIRK_INVALID_BDADDR flag, which can simply be replaced > with HCI_QUIRK_USE_BDADDR_PROPERTY to indicate that the default address > is invalid but can be overridden by firmware (long term, this should > probably just always be allowed). > > Fixes: 6945795bc81a ("Bluetooth: fix use-bdaddr-property quirk") > Cc: stable@vger.kernel.org # 6.5 > Reported-by: Felix Zhang <mrman@mrman314.tech> > Link: https://lore.kernel.org/r/77419ffacc5b4875e920e038332575a2a5bff29f.camel@mrman314.tech/ > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > --- > drivers/bluetooth/hci_bcm4377.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/bluetooth/hci_bcm4377.c b/drivers/bluetooth/hci_bcm4377.c > index a61757835695..9a7243d5db71 100644 > --- a/drivers/bluetooth/hci_bcm4377.c > +++ b/drivers/bluetooth/hci_bcm4377.c > @@ -1417,7 +1417,7 @@ static int bcm4377_check_bdaddr(struct bcm4377_data *bcm4377) > > bda = (struct hci_rp_read_bd_addr *)skb->data; > if (!bcm4377_is_valid_bdaddr(bcm4377, &bda->bdaddr)) > - set_bit(HCI_QUIRK_INVALID_BDADDR, &bcm4377->hdev->quirks); > + set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &bcm4377->hdev->quirks); > > kfree_skb(skb); > return 0; > @@ -2368,7 +2368,6 @@ static int bcm4377_probe(struct pci_dev *pdev, const struct pci_device_id *id) > hdev->set_bdaddr = bcm4377_hci_set_bdaddr; > hdev->setup = bcm4377_hci_setup; > > - set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks); > if (bcm4377->hw->broken_mws_transport_config) > set_bit(HCI_QUIRK_BROKEN_MWS_TRANSPORT_CONFIG, &hdev->quirks); > if (bcm4377->hw->broken_ext_scan) > -- > 2.41.0 > > Looks good to me. This replaces the other patch[1], I take it? Reviewed-by: Neal Gompa <neal@gompa.dev> [1]: https://lore.kernel.org/asahi/aaa107865f4cbd61f8f9006fd3e7ac43b5d1bdad.camel@mrman314.tech/
On Fri, Dec 29, 2023 at 10:11:32AM -0500, Neal Gompa wrote: > On Wed, Dec 27, 2023 at 5:10 AM Johan Hovold <johan+linaro@kernel.org> wrote: > > > > A recent commit restored the original (and still documented) semantics > > for the HCI_QUIRK_USE_BDADDR_PROPERTY quirk so that the device address > > is considered invalid unless an address is provided by firmware. > > > > This specifically means that this flag must only be set for devices with > > invalid addresses, but the Broadcom BCM4377 driver has so far been > > setting this flag unconditionally. > > > > Fortunately the driver already checks for invalid addresses during setup > > and sets the HCI_QUIRK_INVALID_BDADDR flag, which can simply be replaced > > with HCI_QUIRK_USE_BDADDR_PROPERTY to indicate that the default address > > is invalid but can be overridden by firmware (long term, this should > > probably just always be allowed). > > > > Fixes: 6945795bc81a ("Bluetooth: fix use-bdaddr-property quirk") > > Cc: stable@vger.kernel.org # 6.5 > > Reported-by: Felix Zhang <mrman@mrman314.tech> > > Link: https://lore.kernel.org/r/77419ffacc5b4875e920e038332575a2a5bff29f.camel@mrman314.tech/ > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > Looks good to me. This replaces the other patch[1], I take it? Yes, but as Sven explained here, that patch is also correct even though that wasn't obvious from just reading the commit message (which should be amended in my opinion): https://lore.kernel.org/asahi/AB87C916-9CF9-4B8C-AFF5-74CA4151C4FC@svenpeter.dev/ > Reviewed-by: Neal Gompa <neal@gompa.dev> Thanks for reviewing. > [1]: https://lore.kernel.org/asahi/aaa107865f4cbd61f8f9006fd3e7ac43b5d1bdad.camel@mrman314.tech/ Johan
Hello: This patch was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>: On Wed, 27 Dec 2023 11:10:03 +0100 you wrote: > A recent commit restored the original (and still documented) semantics > for the HCI_QUIRK_USE_BDADDR_PROPERTY quirk so that the device address > is considered invalid unless an address is provided by firmware. > > This specifically means that this flag must only be set for devices with > invalid addresses, but the Broadcom BCM4377 driver has so far been > setting this flag unconditionally. > > [...] Here is the summary with links: - Bluetooth: hci_bcm4377: do not mark valid bd_addr as invalid https://git.kernel.org/bluetooth/bluetooth-next/c/cceb1ba62823 You are awesome, thank you!
diff --git a/drivers/bluetooth/hci_bcm4377.c b/drivers/bluetooth/hci_bcm4377.c index a61757835695..9a7243d5db71 100644 --- a/drivers/bluetooth/hci_bcm4377.c +++ b/drivers/bluetooth/hci_bcm4377.c @@ -1417,7 +1417,7 @@ static int bcm4377_check_bdaddr(struct bcm4377_data *bcm4377) bda = (struct hci_rp_read_bd_addr *)skb->data; if (!bcm4377_is_valid_bdaddr(bcm4377, &bda->bdaddr)) - set_bit(HCI_QUIRK_INVALID_BDADDR, &bcm4377->hdev->quirks); + set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &bcm4377->hdev->quirks); kfree_skb(skb); return 0; @@ -2368,7 +2368,6 @@ static int bcm4377_probe(struct pci_dev *pdev, const struct pci_device_id *id) hdev->set_bdaddr = bcm4377_hci_set_bdaddr; hdev->setup = bcm4377_hci_setup; - set_bit(HCI_QUIRK_USE_BDADDR_PROPERTY, &hdev->quirks); if (bcm4377->hw->broken_mws_transport_config) set_bit(HCI_QUIRK_BROKEN_MWS_TRANSPORT_CONFIG, &hdev->quirks); if (bcm4377->hw->broken_ext_scan)