DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org D754C6098A
From:   Kalle Valo <kvalo@codeaurora.org>
To:     Siva Rebbagondla <siva8118@gmail.com>
Cc:     linux-wireless@vger.kernel.org,
        Rishikesh Basu <rishikesh.basu@redpinesignals.com>,
        Siva Rebbagondla <siva.rebbagondla@redpinesignals.com>,
        Ganapathiraju Kondraju <ganapathi.kondraju@redpinesignals.com>,
        krishna.pedda@redpinesignals.com
Subject: Re: [PATCH 2/5] rsi: add hci detach for hibernation and poweroff
References: <1548397802-12180-1-git-send-email-siva8118@gmail.com>
        <1548397802-12180-3-git-send-email-siva8118@gmail.com>
Date:   Mon, 28 Jan 2019 11:29:18 +0200
In-Reply-To: <1548397802-12180-3-git-send-email-siva8118@gmail.com> (Siva
        Rebbagondla's message of "Fri, 25 Jan 2019 11:59:59 +0530")
Message-ID: <87d0oh166p.fsf@codeaurora.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

Siva Rebbagondla <siva8118@gmail.com> writes:

> From: Siva Rebbagondla <siva.rebbagondla@redpinesignals.com>
>
> As we missed to detach HCI, while entering power off or hibernation,
> an extra hci interface gets created whenever system is woken up, to
> avoid this we added hci_detach() in rsi_disconnect(), rsi_freeze(),
> and rsi_shutdown() functions which are invoked for these tests.
> This patch fixes the issue
>
> Signed-off-by: Siva Rebbagondla <siva.rebbagondla@redpinesignals.com>
> ---
>  drivers/net/wireless/rsi/rsi_91x_sdio.c | 21 +++++++++++++++++++--
>  drivers/net/wireless/rsi/rsi_91x_usb.c  |  6 ++++++
>  2 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio.c b/drivers/net/wireless/rsi/rsi_91x_sdio.c
> index b412b65e..59ff778 100644
> --- a/drivers/net/wireless/rsi/rsi_91x_sdio.c
> +++ b/drivers/net/wireless/rsi/rsi_91x_sdio.c
> @@ -1131,7 +1131,12 @@ static void rsi_disconnect(struct sdio_func *pfunction)
>  
>  	rsi_mac80211_detach(adapter);
>  	mdelay(10);
> -
> +#ifdef CONFIG_RSI_COEX
> +	if (adapter->priv->coex_mode > 1 && adapter->priv->bt_adapter) {
> +		rsi_bt_ops.detach(adapter->priv->bt_adapter);
> +		adapter->priv->bt_adapter = NULL;
> +	}
> +#endif

Ifdefs are ugly, avoid them as much as possible. Would this work:

        if (IS_ENABLED(CONFIG_RSI_COEX) &&
            adapter->priv->coex_mode > 1 &&
            adapter->priv->bt_adapter)

BTW, why do you need the ifdef anyway, isn't it enough to check for
coex_mode?


-- 
Kalle Valo