| 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
| ||
|
Message-ID:
<AS4PR04MB96927FD617D139087C38D437E7212@AS4PR04MB9692.eurprd04.prod.outlook.com>
Date: Wed, 20 Nov 2024 04:48:24 +0000
From: Neeraj Sanjay Kale <neeraj.sanjaykale@....com>
To: POPESCU Catalin <catalin.popescu@...ca-geosystems.com>, Sherry Sun
<sherry.sun@....com>, Amitkumar Karwar <amitkumar.karwar@....com>,
"marcel@...tmann.org" <marcel@...tmann.org>, "luiz.dentz@...il.com"
<luiz.dentz@...il.com>, "robh@...nel.org" <robh@...nel.org>,
"krzk+dt@...nel.org" <krzk+dt@...nel.org>, "conor+dt@...nel.org"
<conor+dt@...nel.org>, "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>
CC: "linux-bluetooth@...r.kernel.org" <linux-bluetooth@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"m.felsch@...gutronix.de" <m.felsch@...gutronix.de>,
GEO-CHHER-bsp-development <bsp-development.geo@...ca-geosystems.com>
Subject: Re: [PATCH 2/2] Bluetooth: btnxpuart: implement powerup sequence.
Hi Catalin,
Thank you for the patch. It looks good to me.
> On 06/10/2024 11:04, Sherry Sun wrote:
> > [收到此邮件的某些人通常不会收到来自 sherry.sun@....com
> 的电子邮件。请访问
> >
> https://aka.ms/LearnAboutSenderIdentification，以了解为什么这一点很重
> 要。]
> >
> > This email is not from Hexagon’s Office 365 instance. Please be careful while
> clicking links, opening attachments, or replying to this email.
> >
> >
> >> -----Original Message-----
> >> From: Catalin Popescu <catalin.popescu@...ca-geosystems.com>
> >> Sent: Friday, October 4, 2024 7:36 PM
> >> To: Amitkumar Karwar <amitkumar.karwar@....com>; Neeraj Sanjay Kale
> >> <neeraj.sanjaykale@....com>; marcel@...tmann.org;
> >> luiz.dentz@...il.com; robh@...nel.org; krzk+dt@...nel.org;
> >> conor+dt@...nel.org; p.zabel@...gutronix.de
> >> Cc: linux-bluetooth@...r.kernel.org; devicetree@...r.kernel.org;
> >> linux- kernel@...r.kernel.org; m.felsch@...gutronix.de; bsp-
> >> development.geo@...ca-geosystems.com; Catalin Popescu
> >> <catalin.popescu@...ca-geosystems.com>
> >> Subject: [PATCH 2/2] Bluetooth: btnxpuart: implement powerup sequence.
> >>
> >> NXP bluetooth chip shares power supply and reset gpio with a WLAN chip.
> >> Add support for power supply and reset and enforce powerup
> >> sequence:
> >> 1. apply power supply
> >> 2. deassert reset/powerdown
> > Hi Catalin,
> >
> > For NXP WIFI/BT chip, WIFI and BT share the one PDn pin, which means that
> both wifi and BT controller will be powered on and off at the same time. So
> you control this pin in the BT driver probe and remove function will directly
> break the wifi function I think.
>
> Hi Sherry,
>
> I know and that's why I'm using reset control and not gpio... Reset control are
> refcounted, gpios are not. So, basically we can use one reset control shared by
> bluetooth and wlan drivers, each driver can operate independently from
> other as long as they implement the powerup sequence (shared supply and
> reset pin) and download their specific firmware (combo firmware not used).
> As long as one driver is functional, the chip supply and reset is not asserted.
> So, you can look at the drivers as users of shared resources (supply and
> reset).
>
> >
> >> Signed-off-by: Catalin Popescu <catalin.popescu@...ca-geosystems.com>
> >> ---
> >> drivers/bluetooth/btnxpuart.c | 18 ++++++++++++++++++
> >> 1 file changed, 18 insertions(+)
> >>
> >> diff --git a/drivers/bluetooth/btnxpuart.c
> >> b/drivers/bluetooth/btnxpuart.c index 4f493be763b8..e58e7ac7999f
> >> 100644
> >> --- a/drivers/bluetooth/btnxpuart.c
> >> +++ b/drivers/bluetooth/btnxpuart.c
> >> @@ -16,6 +16,8 @@
> >> #include <linux/crc8.h>
> >> #include <linux/crc32.h>
> >> #include <linux/string_helpers.h>
> >> +#include <linux/regulator/consumer.h> #include <linux/reset.h>
> >>
> >> #include <net/bluetooth/bluetooth.h>
> >> #include <net/bluetooth/hci_core.h> @@ -188,6 +190,7 @@ struct
> >> btnxpuart_dev {
> >>
> >> struct ps_data psdata;
> >> struct btnxpuart_data *nxp_data;
> >> + struct reset_control *pdn;
> >> };
> >>
> >> #define NXP_V1_FW_REQ_PKT 0xa5
> >> @@ -1458,6 +1461,7 @@ static int nxp_serdev_probe(struct
> >> serdev_device
> >> *serdev) {
> >> struct hci_dev *hdev;
> >> struct btnxpuart_dev *nxpdev;
> >> + int err;
> >>
> >> nxpdev = devm_kzalloc(&serdev->dev, sizeof(*nxpdev), GFP_KERNEL);
> >> if (!nxpdev)
> >> @@ -1485,6 +1489,16 @@ static int nxp_serdev_probe(struct
> >> serdev_device
> >> *serdev)
> >>
> >> crc8_populate_msb(crc8_table, POLYNOMIAL8);
> >>
> >> + nxpdev->pdn = devm_reset_control_get_optional_shared(&serdev-
> >>> dev, NULL);
> >> + if (IS_ERR(nxpdev->pdn))
> >> + return PTR_ERR(nxpdev->pdn);
> >> +
> >> + err = devm_regulator_get_enable(&serdev->dev, "vcc");
> >> + if (err) {
> >> + dev_err(&serdev->dev, "Failed to enable vcc regulator\n");
> >> + return err;
> >> + }
> >> +
> > Now in NXP local repo, the PDn pin has been controlled by the
> corresponding wifi PCIe/SDIO controller, so we won't add the vcc-supply and
> reset-gpios properties for the BT driver, seems the code here will return err if
> no vcc-supply and reset-gpios properties provided, which will break the BT
> driver probe.
> If you look closely, the gpio is optional and the supply will return a dummy if
> none is declared in the DT. So, this change won't break anything.
> > Best Regards
> > Sherry
> >
> >> /* Initialize and register HCI device */
> >> hdev = hci_alloc_dev();
> >> if (!hdev) {
> >> @@ -1492,6 +1506,8 @@ static int nxp_serdev_probe(struct
> >> serdev_device
> >> *serdev)
> >> return -ENOMEM;
> >> }
> >>
> >> + reset_control_deassert(nxpdev->pdn);
> >> +
> >> nxpdev->hdev = hdev;
> >>
> >> hdev->bus = HCI_UART;
> >> @@ -1509,6 +1525,7 @@ static int nxp_serdev_probe(struct
> >> serdev_device
> >> *serdev)
> >>
> >> if (hci_register_dev(hdev) < 0) {
> >> dev_err(&serdev->dev, "Can't register HCI device\n");
> >> + reset_control_assert(nxpdev->pdn);
> >> hci_free_dev(hdev);
> >> return -ENODEV;
> >> }
> >> @@ -1540,6 +1557,7 @@ static void nxp_serdev_remove(struct
> >> serdev_device *serdev)
> >> }
> >> ps_cleanup(nxpdev);
> >> hci_unregister_dev(hdev);
> >> + reset_control_assert(nxpdev->pdn);
> >> hci_free_dev(hdev);
> >> }
> >>
> >> --
> >> 2.34.1
> >>
Reviewed-by: Neeraj Sanjay Kale <neeraj.sanjaykale@....com>
Thanks,
Neeraj
Powered by blists - more mailing lists