Re: [PATCH] mt76: sdio: lock sdio when it is needed

From: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
To: sean.wang@mediatek.com
Cc: nbd@nbd.name, 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: Wed, 22 Dec 2021 12:16:29 +0100	[thread overview]
Message-ID: <YcMJDdDrDVIT0Y55@lore-desk> (raw)
In-Reply-To: <7aaf68bc8073c4f1cef063d1e0989e5402ac358c.1640151426.git.objelf@gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 3704 bytes --]

> 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

Regards,
Lorenzo

> +
>  	if (err)
>  		return err;
>  
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c b/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
> index 65d693902c22..743b63f66efa 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c
> @@ -58,7 +58,10 @@ static int mt7921s_parse_intr(struct mt76_dev *dev, struct mt76s_intr *intr)
>  	struct mt7921_sdio_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);
> +
>  	if (err < 0)
>  		return err;
>  
> diff --git a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> index 801590a0a334..f2b46975d831 100644
> --- a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> +++ b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> @@ -102,7 +102,10 @@ mt76s_rx_run_queue(struct mt76_dev *dev, enum mt76_rxq_id qid,
>  
>  	buf = page_address(page);
>  
> +	sdio_claim_host(sdio->func);
>  	err = sdio_readsb(sdio->func, buf, MCR_WRDR(qid), len);
> +	sdio_release_host(sdio->func);
> +
>  	if (err < 0) {
>  		dev_err(dev->dev, "sdio read data failed:%d\n", err);
>  		put_page(page);
> @@ -214,7 +217,10 @@ static int __mt76s_xmit_queue(struct mt76_dev *dev, u8 *data, int len)
>  	if (len > sdio->func->cur_blksize)
>  		len = roundup(len, sdio->func->cur_blksize);
>  
> +	sdio_claim_host(sdio->func);
>  	err = sdio_writesb(sdio->func, MCR_WTDR1, data, len);
> +	sdio_release_host(sdio->func);
> +
>  	if (err)
>  		dev_err(dev->dev, "sdio write failed: %d\n", err);
>  
> @@ -298,6 +304,7 @@ void mt76s_txrx_worker(struct mt76_sdio *sdio)
>  	/* disable interrupt */
>  	sdio_claim_host(sdio->func);
>  	sdio_writel(sdio->func, WHLPCR_INT_EN_CLR, MCR_WHLPCR, NULL);
> +	sdio_release_host(sdio->func);
>  
>  	do {
>  		nframes = 0;
> @@ -327,6 +334,7 @@ void mt76s_txrx_worker(struct mt76_sdio *sdio)
>  	} while (nframes > 0);
>  
>  	/* enable interrupt */
> +	sdio_claim_host(sdio->func);
>  	sdio_writel(sdio->func, WHLPCR_INT_EN_SET, MCR_WHLPCR, NULL);
>  	sdio_release_host(sdio->func);
>  }
> -- 
> 2.25.1
> 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek