Received: by 2002:a25:683:0:0:0:0:0 with SMTP id 125csp2996534ybg; Sat, 6 Jun 2020 07:44:02 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxONIV3YEYl3z7+7oTYqaUvSrczWa6IDcVldBFO+cr+ImsbxFAYOgFHqfZKltbwE+UoJdwZ X-Received: by 2002:a50:fc0d:: with SMTP id i13mr13828869edr.260.1591454642461; Sat, 06 Jun 2020 07:44:02 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1591454642; cv=none; d=google.com; s=arc-20160816; b=LeL+/SahaC5bbbmamg7/GzGXmWhHgFqW7o1qz5tJ+5lMmkv7cHaghHSWEB4nS5A0FY 3RcjwoaETxW46aWBwC1j0wuN47F2I6aDWoYvnGZ030mu82fF1T1KaVvRUDqEiGzBMtu4 0dEyULfFHElSwYjC9Spyci3JclshyFleIgVqor0UJdq28gH0lJtrPK7EJMO+Vb/qb5Oo ucrX3GAGAjW8b1chR7EwjfV12xOiQNig2VETJ3evLtiSHYQPTyrL4yn22dFoU/WBr9Ye qzQE+jPCKXjEdhvFZYD2KiNlk9/acQqYElugm/jnqlEhJYjB71yndQEakgkq3q54nG+A l8+w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:in-reply-to:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :dkim-signature; bh=uRszDx+SWbrUSA1S9dvL1khB1G3QVBSQcKh5Peg+R5c=; b=qsmNuAyyoXOKFeGW2wR3YP9ghgCt+rz49WOZqIDCp62UiJ+6X75i/nV067oUkbXHPb dk/6WKjEaDlgCroGf8pRMNUUau3wV6gu1spWYIgqOY7U5j671RuW83wtIgPMFM6TQAPI ePL7bHHMevyN2wsPGpETWzNDTgaFDvr4eUuUnAPZj6fDlDs/UOfm26OzVA1lFgxDGgOC h5fWkckFHW24oidoWQaJxnYOCZTi+CGb74irsW4X3Ta0B0eUCZ919mMdOOf/NeH86sQM GOzWXQI54No36JTUVrgamKUqijjZKxw948AhO0hmwNma5jbbnvEemBIqAtJhWGEteVrV 0c1g== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=kHRhckqz; spf=pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-bluetooth-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id 17si5582928edw.300.2020.06.06.07.43.16; Sat, 06 Jun 2020 07:44:02 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=kHRhckqz; spf=pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-bluetooth-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728775AbgFFOms (ORCPT + 99 others); Sat, 6 Jun 2020 10:42:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50566 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727104AbgFFOms (ORCPT ); Sat, 6 Jun 2020 10:42:48 -0400 Received: from mail-pl1-x643.google.com (mail-pl1-x643.google.com [IPv6:2607:f8b0:4864:20::643]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0F33BC03E96A for ; Sat, 6 Jun 2020 07:42:48 -0700 (PDT) Received: by mail-pl1-x643.google.com with SMTP id m7so4875264plt.5 for ; Sat, 06 Jun 2020 07:42:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=uRszDx+SWbrUSA1S9dvL1khB1G3QVBSQcKh5Peg+R5c=; b=kHRhckqzf6lsVhYRZE5hxpsmQRw3lkGndVmIteEJTmWrJsvPThCy+KhTwgBNtITuJf TjOJtWMig5GLG3otDAPq/wRJHWobLieiuSKjpp8m4lhOnnDA52i/6+ARiHyE8zTpcc1a AbM/KA6MUJKRhHqYyPI0OcMBcSVTs+93kK3LI= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=uRszDx+SWbrUSA1S9dvL1khB1G3QVBSQcKh5Peg+R5c=; b=mUzNMiayBpSk7yGiR5+nS8r5A7tqGAQ05Ki2phmTFBxayc+VRExhMjKmA1DkUCiLeS wdEXr67v5IZgkvrxrPS8+mIcqzNOt1TvvaKUbZZ3CnaX1880y9VpP57nm0QH+s4PjrOL vGg8gBz7IKqHpdWQbEewb7qUo8AfmDrSLRpxhAGfUcF2cbK19VnZLP9UkF39sxm0m/h2 RxS+JONDy3/htj5jue8R0/eMaURRgXcsiiVxkF5x+5qjp5TPzEf44l2bz4iGxUOLNCAa IZoCYxQCCfqCxUSDPgoejl/DV6y3RA3t4ybRlqgFsHX4mSI7bnjpFVuCr4/iFt+AAJ24 0ziQ== X-Gm-Message-State: AOAM5330c0mYGuHw6wk2Li9d2ylxgAohP8p/Nz+xUJQaiQNwB/02TBfH 7qiTrJD1BVliCisvsiYNzduwHg== X-Received: by 2002:a17:902:a412:: with SMTP id p18mr14119856plq.111.1591454567480; Sat, 06 Jun 2020 07:42:47 -0700 (PDT) Received: from localhost ([2620:15c:202:1:4fff:7a6b:a335:8fde]) by smtp.gmail.com with ESMTPSA id n24sm10638960pjt.47.2020.06.06.07.42.46 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 06 Jun 2020 07:42:46 -0700 (PDT) Date: Sat, 6 Jun 2020 07:42:45 -0700 From: Matthias Kaehlcke To: bgodavar@codeaurora.org Cc: Marcel Holtmann , Johan Hedberg , linux-bluetooth@vger.kernel.org, Rocky Liao , Zijun Hu , linux-kernel@vger.kernel.org, Abhishek Pandit-Subedi , Claire Chang , hemantg@codeaurora.org Subject: Re: [PATCH 2/3] Bluetooth: hci_qca: Skip serdev wait when no transfer is pending Message-ID: <20200606144245.GQ4525@google.com> References: <20200605184611.252218-1-mka@chromium.org> <20200605114552.2.I2a095adb2a9a98b15c11d7310db142b27f8cab28@changeid> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Sender: linux-bluetooth-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org Hi Bala, On Sat, Jun 06, 2020 at 06:27:59PM +0530, bgodavar@codeaurora.org wrote: > Hi matthias, > > On 2020-06-06 00:16, Matthias Kaehlcke wrote: > > qca_suspend() calls serdev_device_wait_until_sent() regardless of > > whether a transfer is pending. While it does no active harm since > > the function should return immediately it makes the code more > > confusing. Add a flag to track whether a transfer is pending and > > only call serdev_device_wait_until_sent() is needed. > > > > Signed-off-by: Matthias Kaehlcke > > --- > > > > drivers/bluetooth/hci_qca.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c > > index b1d82d32892e9..90ffd8ca1fb0d 100644 > > --- a/drivers/bluetooth/hci_qca.c > > +++ b/drivers/bluetooth/hci_qca.c > > @@ -2050,6 +2050,7 @@ static int __maybe_unused qca_suspend(struct > > device *dev) > > struct hci_uart *hu = &qcadev->serdev_hu; > > struct qca_data *qca = hu->priv; > > unsigned long flags; > > + bool tx_pending = false; > > int ret = 0; > > u8 cmd; > > > > @@ -2083,6 +2084,7 @@ static int __maybe_unused qca_suspend(struct > > device *dev) > > > > qca->tx_ibs_state = HCI_IBS_TX_ASLEEP; > > qca->ibs_sent_slps++; > > + tx_pending = true; > > break; > > > > case HCI_IBS_TX_ASLEEP: > > @@ -2099,8 +2101,10 @@ static int __maybe_unused qca_suspend(struct > > device *dev) > > if (ret < 0) > > goto error; > > > > - serdev_device_wait_until_sent(hu->serdev, > > - msecs_to_jiffies(CMD_TRANS_TIMEOUT_MS)); > > + if (tx_pending) { > [Bala]: Good idea why don't we move this call to switch case under > HCI_IBS_TX_AWAKE > https://elixir.bootlin.com/linux/latest/source/drivers/bluetooth/hci_qca.c#L1994 I agree that this would make the code easier to follow, however the reason this wasn't done in the first place is (probably) that the IBS spinlock is held during the switch/case block. In principle the unlock could be done before calling _wait_until_sent(), but then the unlock also needs to happen in the other 'case' branches. It's not ideal, but also not necessarily worse than introducing 'tx_pending', the repeated unlocks might be preferable in terms of readability. > i.e. i would recommend below sequence > > 1. Send SLEEP BYTE > 2. wait for some time to write SLEEP Byte on Tx line > 3. call for Tx clock off qca_wq_serial_tx_clock_vote_off > > > + serdev_device_wait_until_sent(hu->serdev, > > + msecs_to_jiffies(CMD_TRANS_TIMEOUT_MS)); > > + } > > > > /* Wait for HCI_IBS_SLEEP_IND sent by device to indicate its Tx is > > going > > * to sleep, so that the packet does not wake the system later.