linux-kernel - Re: [PATCH v1 2/2] Bluetooth: btnxpuart: Fix nxp

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]

Message-ID: <caefad296d06503c69a095376acf3f65070604fa.camel@toradex.com>
Date:   Fri, 20 Oct 2023 07:56:12 +0000
From:   Marcel Ziswiler <marcel.ziswiler@...adex.com>
To:     "pmenzel@...gen.mpg.de" <pmenzel@...gen.mpg.de>
CC:     "sherry.sun@....com" <sherry.sun@....com>,
        "luiz.dentz@...il.com" <luiz.dentz@...il.com>,
        "marcel@...tmann.org" <marcel@...tmann.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "johan.hedberg@...il.com" <johan.hedberg@...il.com>,
        "linux-bluetooth@...r.kernel.org" <linux-bluetooth@...r.kernel.org>,
        "amitkumar.karwar@....com" <amitkumar.karwar@....com>,
        "neeraj.sanjaykale@....com" <neeraj.sanjaykale@....com>,
        "ilpo.jarvinen@...ux.intel.com" <ilpo.jarvinen@...ux.intel.com>
Subject: Re: [PATCH v1 2/2] Bluetooth: btnxpuart: Fix nxp_setup

Hi Paul

On Thu, 2023-10-19 at 18:44 +0200, Paul Menzel wrote:
> Dear Marcel,
> 
> 
> Thank you for your patch.

Thanks for your feedback.

> Am 18.10.23 um 16:55 schrieb Marcel Ziswiler:
> > From: Marcel Ziswiler <marcel.ziswiler@...adex.com>
> 
> “Fix nxp_setup” is really generic. A more specific message would be 
> great. Maybe: Wait up to 10 s for firmware.

Okay, I can do that.

I admit, I had a hard time finding a real good description as the whole thing is rather completely flawed.

> > Unfortunately, nxp_setup() may inadvertently assume that the
> > firmware is already running while the module is not even powered yet.
> > Fix this by waiting up to 10 seconds for the CTS to go up as the combo
> > firmware might be loaded by the Wi-Fi driver over SDIO (mwifiex_sdio).
> > 
> > Fixes: 689ca16e5232 ("Bluetooth: NXP: Add protocol support for NXP Bluetooth chipsets")
> > 
> > Signed-off-by: Marcel Ziswiler <marcel.ziswiler@...adex.com>
> > 
> > ---
> > This is what may happen without this fix:
> > [  284.588177] Bluetooth: hci0: Opcode 0x0c03 failed: -110
> > [  286.636167] Bluetooth: hci0: Setting wake-up method failed (-110)
> > Unfortunately, even re-loading the btnxpuart kernel module would not
> > recover from this condition.
> 
> I’d add that information to the commit message.

Okay, will do.

> >   drivers/bluetooth/btnxpuart.c | 10 ++++++++++
> >   1 file changed, 10 insertions(+)
> > 
> > diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
> > index 9cb7529eef09..4b83a0aa3459 100644
> > --- a/drivers/bluetooth/btnxpuart.c
> > +++ b/drivers/bluetooth/btnxpuart.c
> > @@ -1021,6 +1021,16 @@ static int nxp_setup(struct hci_dev *hdev)
> >   		if (err < 0)
> >   			return err;
> >   	} else {
> > +		/* The combo firmware might be loaded by the Wi-Fi driver over SDIO (mwifiex_sdio).
> > +		 * We wait up to 10s for the CTS to go up. Afterwards, we know that the firmware is
> > +		 * really ready.
> > +		 */
> > +		err = serdev_device_wait_for_cts(nxpdev->serdev, true, 10000);
> > +		if (err) {
> > +			bt_dev_err(nxpdev->hdev, "Wait for CTS failed with %d", err);
> > +			return err;
> > +		}
> > +
> >   		bt_dev_dbg(hdev, "FW already running.");
> >   		clear_bit(BTNXPUART_FW_DOWNLOADING, &nxpdev->tx_state);
> >   	}
> 
> Isn’t there another way to find out, if the firmware is there? Adding an 
> arbitrary timeout of ten seconds sounds like a fundamental flaw in the 
> driver?

Yes, exactly. You might have read my previous message [1]. The whole driver/firmware, well the entire thing is
basically rather flawed. But then I am just trying to incrementally make it at least half-way usable for our
use case as well.

[1] https://lore.kernel.org/all/dca8bc7fec5f527cac2e280cd8ed4edae1f473ea.camel@toradex.com

> Kind regards,
> 
> Paul

Cheers

Marcel