Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 4 Apr 2023 09:11:29 +0200
From:   Sascha Hauer <s.hauer@pengutronix.de>
To:     Jonathan Bither <jonbither@gmail.com>
Cc:     linux-wireless <linux-wireless@vger.kernel.org>,
        Hans Ulli Kroll <linux@ulli-kroll.de>,
        Larry Finger <Larry.Finger@lwfinger.net>,
        Pkshih <pkshih@realtek.com>, Tim K <tpkuester@gmail.com>,
        "Alex G ." <mr.nuke.me@gmail.com>,
        Nick Morrow <morrownr@gmail.com>,
        Viktor Petrenko <g0000ga@gmail.com>,
        Andreas Henriksson <andreas@fatal.se>,
        ValdikSS <iam@valdikss.org.ru>, kernel@pengutronix.de,
        stable@vger.kernel.org
Subject: Re: [PATCH 1/2] wifi: rtw88: usb: fix priority queue to endpoint
 mapping
Message-ID: <20230404071129.GW19113@pengutronix.de>
References: <20230331121054.112758-1-s.hauer@pengutronix.de>
 <20230331121054.112758-2-s.hauer@pengutronix.de>
 <cb5980b4-1e0e-57f8-e680-44e14fa0b02c@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <cb5980b4-1e0e-57f8-e680-44e14fa0b02c@gmail.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On Fri, Mar 31, 2023 at 10:31:25AM -0400, Jonathan Bither wrote:
> 
> On 3/31/23 08:10, Sascha Hauer wrote:
> > The RTW88 chipsets have four different priority queues in hardware. For
> > the USB type chipsets the packets destined for a specific priority queue
> > must be sent through the endpoint corresponding to the queue. This was
> > not fully understood when porting from the RTW88 USB out of tree driver
> > and thus violated.
> > 
> > This patch implements the qsel to endpoint mapping as in
> > get_usb_bulkout_id_88xx() in the downstream driver.
> > 
> > Without this the driver often issues "timed out to flush queue 3"
> > warnings and often TX stalls completely.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >   drivers/net/wireless/realtek/rtw88/usb.c | 70 ++++++++++++++++--------
> >   1 file changed, 47 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
> > index 2a8336b1847a5..a10d6fef4ffaf 100644
> > --- a/drivers/net/wireless/realtek/rtw88/usb.c
> > +++ b/drivers/net/wireless/realtek/rtw88/usb.c
> > @@ -118,6 +118,22 @@ static void rtw_usb_write32(struct rtw_dev *rtwdev, u32 addr, u32 val)
> >   	rtw_usb_write(rtwdev, addr, val, 4);
> >   }
> > +static int dma_mapping_to_ep(enum rtw_dma_mapping dma_mapping)
> > +{
> > +	switch (dma_mapping) {
> > +	case RTW_DMA_MAPPING_HIGH:
> > +		return 0;
> > +	case RTW_DMA_MAPPING_NORMAL:
> > +		return 1;
> > +	case RTW_DMA_MAPPING_LOW:
> > +		return 2;
> > +	case RTW_DMA_MAPPING_EXTRA:
> > +		return 3;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> Would it be beneficial to use defines for the returns? Would the
> USB_ENDPOINT_XFER_ defines be applicable?

The USB_ENDPOINT_XFER_* macros encode the type of the transfer, like
bulk, control, isochronous and interrupt. What I need here really is
the endpoint number. I don't see a benefit in adding a define here.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |