RE: [PATCH v1 3/3] Bluetooth: hci_qca: WAR to handle WCN6750 HW issue

["Balakrishna Godavarthi (QUIC)" <quic_bgodavar@quicinc.com>]

Hi Marcel,

-----Original Message-----
From: Marcel Holtmann <marcel@holtmann.org> 
Sent: Friday, April 29, 2022 8:24 PM
To: Balakrishna Godavarthi (QUIC) <quic_bgodavar@quicinc.com>
Cc: Andy Gross <agross@kernel.org>; robh+dt@kernel.org; bjorn.andersson@linaro.org; linux-arm-msm@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; Johan Hedberg <johan.hedberg@gmail.com>; mka@chromium.org; linux-bluetooth@vger.kernel.org; Hemant Gupta (QUIC) <quic_hemantg@quicinc.com>; Sai Teja Aluvala (Temp) (QUIC) <quic_saluvala@quicinc.com>; quic_rjliao <quic_rjliao@quicinc.com>; mcchou@chromium.org
Subject: Re: [PATCH v1 3/3] Bluetooth: hci_qca: WAR to handle WCN6750 HW issue

Hi Balakrishna,

> The patch is workaround for hardware issue on WCN6750.
> On WCN6750 sometimes observed AON power source takes 100ms time to 
> fully discharge voltage during OFF. As WCN6750 is combo chip for WLAN 
> and BT. If any of the tech area ON is triggered during discharge 
> phase, it fails to turn ON.
> To overcome this hardware issue, During BT ON, driver check for 
> WLAN_EN pin status. If it high, it will pull BT_EN to high immediately 
> else it will wait for 100ms assuming WLAN was just powered OFF and 
> then BT_EN will be pulled to high.
> 
> Fixes: d8f97da1b92d2 ("Bluetooth: hci_qca: Add support for QTI 
> Bluetooth chip wcn6750")
> Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
> Signed-off-by: Sai Teja Aluvala <quic_saluvala@quicinc.com>
> Signed-off-by: Balakrishna Godavarthi <quic_bgodavar@quicinc.com>
> ---
> drivers/bluetooth/hci_qca.c | 30 ++++++++++++++++++++++++------
> 1 file changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c 
> index eab34e2..c3862d1 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -219,6 +219,7 @@ struct qca_serdev {
> 	struct hci_uart	 serdev_hu;
> 	struct gpio_desc *bt_en;
> 	struct gpio_desc *sw_ctrl;
> +	struct gpio_desc *wlan_en;
> 	struct clk	 *susclk;
> 	enum qca_btsoc_type btsoc_type;
> 	struct qca_power *bt_power;

I am really against these intermixing of Bluetooth and WiFi details. There is work ongoing to do some sequence power procedure. Maybe that is something you should look into. This is a mess.

And again, we are still hacking around hci_qca.c instead of writing a clean serdev only driver for this hardware. I have the feeling that nobody listens to review comments these days. It is just hacking patches together to get hardware enabled somehow and then disappear.

[Bala]: We are working on serdev like driver for our QCA platform.  We are in initial stages of discussion, and soon we will start the driver development work. 
In mean to stop stability or functional issues we are trying to add these HACKs in QCA driver.

Regards

Marcel