From:   <sean.wang@mediatek.com>
To:     <lorenzo.bianconi@redhat.com>
CC:     <nbd@nbd.name>, <sean.wang@mediatek.com>,
        <Soul.Huang@mediatek.com>, <YN.Chen@mediatek.com>,
        <Leon.Yen@mediatek.com>, <Eric-SY.Chang@mediatek.com>,
        <Mark-YW.Chen@mediatek.com>, <Deren.Wu@mediatek.com>,
        <km.lin@mediatek.com>, <jenhao.yang@mediatek.com>,
        <robin.chiu@mediatek.com>, <Eddie.Chen@mediatek.com>,
        <ch.yeh@mediatek.com>, <posh.sun@mediatek.com>,
        <ted.huang@mediatek.com>, <Eric.Liang@mediatek.com>,
        <Stella.Chang@mediatek.com>, <Tom.Chou@mediatek.com>,
        <steve.lee@mediatek.com>, <jsiuda@google.com>,
        <frankgor@google.com>, <jemele@google.com>,
        <abhishekpandit@google.com>, <shawnku@google.com>,
        <linux-wireless@vger.kernel.org>,
        <linux-mediatek@lists.infradead.org>
Subject: Re: [PATCH] mt76: sdio: lock sdio when it is needed
Date:   Thu, 23 Dec 2021 03:27:28 +0800
Message-ID: <1640201248-14901-1-git-send-email-sean.wang@mediatek.com>
In-Reply-To: <YcMJDdDrDVIT0Y55@lore-desk--annotate>
References: <YcMJDdDrDVIT0Y55@lore-desk--annotate>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

From: Sean Wang <sean.wang@mediatek.com>

>> From: Sean Wang <sean.wang@mediatek.com>
>>
>> Acquire the SDIO as needed as possible because either MT7663S or
>> MT7921S is a multiple-function device that always includes Bluetooth
>> that would share with the same SDIO bus. So not to avoid breaking
>> Bluetooth pairing, audio, and HID such kind of time critical
>> application on that, we only lock sdio bus when it is necessary in WiFi driver.
>>
>> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
>> ---
>>  drivers/net/wireless/mediatek/mt76/mt7615/sdio.c | 3 +++
>> drivers/net/wireless/mediatek/mt76/mt7921/sdio.c | 3 +++
>>  drivers/net/wireless/mediatek/mt76/sdio_txrx.c   | 8 ++++++++
>>  3 files changed, 14 insertions(+)
>>
>> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/sdio.c
>> b/drivers/net/wireless/mediatek/mt76/mt7615/sdio.c
>> index 31c4a76b7f91..71162befdae8 100644
>> --- a/drivers/net/wireless/mediatek/mt76/mt7615/sdio.c
>> +++ b/drivers/net/wireless/mediatek/mt76/mt7615/sdio.c
>> @@ -56,7 +56,10 @@ static int mt7663s_parse_intr(struct mt76_dev *dev, struct mt76s_intr *intr)
>>	struct mt7663s_intr *irq_data = sdio->intr_data;
>>	int i, err;
>>
>> +	sdio_claim_host(sdio->func);
>>	err = sdio_readsb(sdio->func, irq_data, MCR_WHISR,
>> sizeof(*irq_data));
>> +	sdio_release_host(sdio->func);
>
>I guess you can move this in mt76s_rx_handler() and remove the duplicated code in mt7921_sdio.c
>

I got your point, but I still prefer we can release sdio lock as early as possible when WiFi driver doesn't need it
to allow BT driver has the chance to get the lock quickly.

>Regards,
>Lorenzo
>
>> +
>>	if (err)
>>		return err;

<snip>