| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20241107233403.6li5oawn6d23e6gf@synopsys.com>
Date: Thu, 7 Nov 2024 23:34:11 +0000
From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To: Selvarasu Ganesan <selvarasu.g@...sung.com>
CC: Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"quic_akakum@...cinc.com" <quic_akakum@...cinc.com>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"jh0801.jung@...sung.com" <jh0801.jung@...sung.com>,
"dh10.jung@...sung.com" <dh10.jung@...sung.com>,
"naushad@...sung.com" <naushad@...sung.com>,
"akash.m5@...sung.com" <akash.m5@...sung.com>,
"rc93.raju@...sung.com" <rc93.raju@...sung.com>,
"taehyun.cho@...sung.com" <taehyun.cho@...sung.com>,
"hongpooh.kim@...sung.com" <hongpooh.kim@...sung.com>,
"eomji.oh@...sung.com" <eomji.oh@...sung.com>,
"shijie.cai@...sung.com" <shijie.cai@...sung.com>,
"stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: Re: [PATCH] usb: dwc3: gadget: Add TxFIFO resizing supports for
single port RAM
On Thu, Nov 07, 2024, Selvarasu Ganesan wrote:
> This commit adds support for resizing the TxFIFO in USB2.0-only mode
> where using single port RAM, and limit the use of extra FIFOs for bulk
This should be split into 2 changes: 1 for adding support for
single-port RAM, and the other for budgeting the bulk fifo setting.
The first change is not a fix, and the latter may be a fix (may need
more feedback from others).
> transfers in non-SS mode. It prevents the issue of limited RAM size
> usage.
>
> Fixes: fad16c823e66 ("usb: dwc3: gadget: Refine the logic for resizing Tx FIFOs")
> Cc: stable@...r.kernel.org # 6.12.x: fad16c82: usb: dwc3: gadget: Refine the logic for resizing Tx FIFOs
> Signed-off-by: Selvarasu Ganesan <selvarasu.g@...sung.com>
> ---
> drivers/usb/dwc3/core.h | 4 +++
> drivers/usb/dwc3/gadget.c | 56 ++++++++++++++++++++++++++++++---------
> 2 files changed, 48 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index eaa55c0cf62f..8306b39e5c64 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -915,6 +915,7 @@ struct dwc3_hwparams {
> #define DWC3_MODE(n) ((n) & 0x7)
>
> /* HWPARAMS1 */
> +#define DWC3_SPRAM_TYPE(n) (((n) >> 23) & 1)
> #define DWC3_NUM_INT(n) (((n) & (0x3f << 15)) >> 15)
>
> /* HWPARAMS3 */
> @@ -925,6 +926,9 @@ struct dwc3_hwparams {
> #define DWC3_NUM_IN_EPS(p) (((p)->hwparams3 & \
> (DWC3_NUM_IN_EPS_MASK)) >> 18)
>
> +/* HWPARAMS6 */
> +#define DWC3_RAM0_DEPTH(n) (((n) & (0xffff0000)) >> 16)
> +
> /* HWPARAMS7 */
> #define DWC3_RAM1_DEPTH(n) ((n) & 0xffff)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 2fed2aa01407..d3e25f7d7cd0 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -687,6 +687,42 @@ static int dwc3_gadget_calc_tx_fifo_size(struct dwc3 *dwc, int mult)
> return fifo_size;
> }
>
> +/**
> + * dwc3_gadget_calc_ram_depth - calculates the ram depth for txfifo
> + * @dwc: pointer to the DWC3 context
> + */
> +static int dwc3_gadget_calc_ram_depth(struct dwc3 *dwc)
> +{
> + int ram_depth;
> + int fifo_0_start;
> + bool spram_type;
> + int tmp;
> +
> + /* Check supporting RAM type by HW */
> + spram_type = DWC3_SPRAM_TYPE(dwc->hwparams.hwparams1);
> +
> + /* If a single port RAM is utilized, then allocate TxFIFOs from
> + * RAM0. otherwise, allocate them from RAM1.
> + */
Please use this comment block style
/*
* xxxx
* xxxx
*/
> + ram_depth = spram_type ? DWC3_RAM0_DEPTH(dwc->hwparams.hwparams6) :
> + DWC3_RAM1_DEPTH(dwc->hwparams.hwparams7);
Don't use spram_type as a boolean. Perhaps define a macro for type value
1 and 0 (for single vs 2-port)
> +
> +
> + /* In a single port RAM configuration, the available RAM is shared
> + * between the RX and TX FIFOs. This means that the txfifo can begin
> + * at a non-zero address.
> + */
> + if (spram_type) {
if (spram_type == DWC3_SPRAM_TYPE_SINGLE) {
...
}
> + /* Check if TXFIFOs start at non-zero addr */
> + tmp = dwc3_readl(dwc->regs, DWC3_GTXFIFOSIZ(0));
> + fifo_0_start = DWC3_GTXFIFOSIZ_TXFSTADDR(tmp);
> +
> + ram_depth -= (fifo_0_start >> 16);
> + }
> +
> + return ram_depth;
> +}
> +
> /**
> * dwc3_gadget_clear_tx_fifos - Clears txfifo allocation
> * @dwc: pointer to the DWC3 context
> @@ -753,7 +789,7 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep)
> {
> struct dwc3 *dwc = dep->dwc;
> int fifo_0_start;
> - int ram1_depth;
> + int ram_depth;
> int fifo_size;
> int min_depth;
> int num_in_ep;
> @@ -773,7 +809,7 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep)
> if (dep->flags & DWC3_EP_TXFIFO_RESIZED)
> return 0;
>
> - ram1_depth = DWC3_RAM1_DEPTH(dwc->hwparams.hwparams7);
> + ram_depth = dwc3_gadget_calc_ram_depth(dwc);
>
> switch (dwc->gadget->speed) {
> case USB_SPEED_SUPER_PLUS:
> @@ -792,10 +828,6 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep)
> break;
> }
> fallthrough;
> - case USB_SPEED_FULL:
> - if (usb_endpoint_xfer_bulk(dep->endpoint.desc))
> - num_fifos = 2;
> - break;
Please take out the fallthrough above if you remove this condition.
> default:
> break;
> }
> @@ -809,7 +841,7 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep)
>
> /* Reserve at least one FIFO for the number of IN EPs */
> min_depth = num_in_ep * (fifo + 1);
> - remaining = ram1_depth - min_depth - dwc->last_fifo_depth;
> + remaining = ram_depth - min_depth - dwc->last_fifo_depth;
> remaining = max_t(int, 0, remaining);
> /*
> * We've already reserved 1 FIFO per EP, so check what we can fit in
> @@ -835,9 +867,9 @@ static int dwc3_gadget_resize_tx_fifos(struct dwc3_ep *dep)
> dwc->last_fifo_depth += DWC31_GTXFIFOSIZ_TXFDEP(fifo_size);
>
> /* Check fifo size allocation doesn't exceed available RAM size. */
> - if (dwc->last_fifo_depth >= ram1_depth) {
> + if (dwc->last_fifo_depth >= ram_depth) {
> dev_err(dwc->dev, "Fifosize(%d) > RAM size(%d) %s depth:%d\n",
> - dwc->last_fifo_depth, ram1_depth,
> + dwc->last_fifo_depth, ram_depth,
> dep->endpoint.name, fifo_size);
> if (DWC3_IP_IS(DWC3))
> fifo_size = DWC3_GTXFIFOSIZ_TXFDEP(fifo_size);
> @@ -3090,7 +3122,7 @@ static int dwc3_gadget_check_config(struct usb_gadget *g)
> struct dwc3 *dwc = gadget_to_dwc(g);
> struct usb_ep *ep;
> int fifo_size = 0;
> - int ram1_depth;
> + int ram_depth;
> int ep_num = 0;
>
> if (!dwc->do_fifo_resize)
> @@ -3113,8 +3145,8 @@ static int dwc3_gadget_check_config(struct usb_gadget *g)
> fifo_size += dwc->max_cfg_eps;
>
> /* Check if we can fit a single fifo per endpoint */
> - ram1_depth = DWC3_RAM1_DEPTH(dwc->hwparams.hwparams7);
> - if (fifo_size > ram1_depth)
> + ram_depth = dwc3_gadget_calc_ram_depth(dwc);
> + if (fifo_size > ram_depth)
> return -ENOMEM;
>
> return 0;
> --
> 2.17.1
>
We may need to think a little more on how to budgeting the resource
properly to accomodate for different requirements. If there's no single
formula to satisfy for all platform, perhaps we may need to introduce
parameters that users can set base on the needs of their application.
I'd like to Ack on the new change that checks single-port RAM. For the
budgeting of fifo, let's keep the discussion going a little more.
Thanks,
Thinh
Powered by blists - more mailing lists