Date:   Wed, 22 Dec 2021 12:16:29 +0100
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
Message-ID: <YcMJDdDrDVIT0Y55@lore-desk>
References: <7aaf68bc8073c4f1cef063d1e0989e5402ac358c.1640151426.git.objelf@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="QoOniBAhkIvv76Ai"
Content-Disposition: inline
In-Reply-To: <7aaf68bc8073c4f1cef063d1e0989e5402ac358c.1640151426.git.objelf@gmail.com>
Precedence: bulk


--QoOniBAhkIvv76Ai
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

> From: Sean Wang <sean.wang@mediatek.com>
>=20
> 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.
>=20
> 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(+)
>=20
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/sdio.c b/drivers/n=
et/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, st=
ruct mt76s_intr *intr)
>  	struct mt7663s_intr *irq_data =3D sdio->intr_data;
>  	int i, err;
> =20
> +	sdio_claim_host(sdio->func);
>  	err =3D 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 c=
ode
in mt7921_sdio.c

Regards,
Lorenzo

> +
>  	if (err)
>  		return err;
> =20
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/sdio.c b/drivers/n=
et/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, st=
ruct mt76s_intr *intr)
>  	struct mt7921_sdio_intr *irq_data =3D sdio->intr_data;
>  	int i, err;
> =20
> +	sdio_claim_host(sdio->func);
>  	err =3D sdio_readsb(sdio->func, irq_data, MCR_WHISR, sizeof(*irq_data));
> +	sdio_release_host(sdio->func);
> +
>  	if (err < 0)
>  		return err;
> =20
> 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_r=
xq_id qid,
> =20
>  	buf =3D page_address(page);
> =20
> +	sdio_claim_host(sdio->func);
>  	err =3D 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 =3D roundup(len, sdio->func->cur_blksize);
> =20
> +	sdio_claim_host(sdio->func);
>  	err =3D 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);
> =20
> @@ -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);
> =20
>  	do {
>  		nframes =3D 0;
> @@ -327,6 +334,7 @@ void mt76s_txrx_worker(struct mt76_sdio *sdio)
>  	} while (nframes > 0);
> =20
>  	/* enable interrupt */
> +	sdio_claim_host(sdio->func);
>  	sdio_writel(sdio->func, WHLPCR_INT_EN_SET, MCR_WHLPCR, NULL);
>  	sdio_release_host(sdio->func);
>  }
> --=20
> 2.25.1
>=20

--QoOniBAhkIvv76Ai
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQTquNwa3Txd3rGGn7Y6cBh0uS2trAUCYcMJDQAKCRA6cBh0uS2t
rNBHAP90xJ+Mada+OCDe3sgAR7bOZEJiGQbcCDVAyv0qDwuBdwD7B2xzkTpTv7Tk
p/MUb4rB7hR+2QQQfaJbmPXIH3Jd6QQ=
=J2gf
-----END PGP SIGNATURE-----

--QoOniBAhkIvv76Ai--