All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pavel Hofman <pavel.hofman@ivitera.com>
To: Stefan Wahren <stefan.wahren@i2se.com>,
	Minas Harutyunyan <hminas@synopsys.com>,
	Rob Herring <robh+dt@kernel.org>,
	Nicolas Saenz Julienne <nsaenz@kernel.org>,
	linux-usb@vger.kernel.org, devicetree@vger.kernel.org,
	linux-rpi-kernel@lists.infradead.org
Subject: Re: [PATCH] ARM: dts: bcm283x: increase dwc2's RX FIFO size
Date: Fri, 28 May 2021 10:59:30 +0200	[thread overview]
Message-ID: <a5b12552-1340-aa71-caca-fbef98b8b3e3@ivitera.com> (raw)
In-Reply-To: <f9c90203-a67e-0e33-09a8-f173af63e771@i2se.com>

Dne 27. 05. 21 v 15:47 Stefan Wahren napsal(a):

>> I think I see the problem.
>>
>> IIUC the calculations and checks, all g-tx-fifo-size values +
>> g-rx-fifo-size + g-np-tx-fifo-size must not exceed total_fifo_size. My
>> RPi4 reports the total_fifo_size as 4080 (in
>> /sys/kernel/debug/usb/fe980000.usb/hw_params).
>>
>> Linux mainline
>> https://github.com/torvalds/linux/search?p=3&q=g-tx-fifo-size :
>>
>> The increase in value of g-rx-fifo-size exceeds the limit for the DTSI
>> files we patched:
>>
>> Both bcm283x-rpi-usb-peripheral.dtsi and bcm283x-rpi-usb-otg.dtsi:
>> 558 + 32 + 256 + 256 + 512 + 512 + 512 + 768 + 768 = 4174 > 4080
>>
>> while the sum with the previous value of 256 reached just 3872 < 4080.
>>
>>
>> The raspberrypi repo
>> https://github.com/raspberrypi/linux/search?q=g-tx-fifo-size :
>>
>> It has a different mix of the DTSI files
>> dwc2-overlay.dts
>> upstream-overlay.dts
>> upstream-pi4-overlay.dts
> yes these overlay files are vendor specific and doesn't exist in
> mainline. The upstream*dts were intended to "simulate" mainline
> behavior, but unfortunately differ in this case.
>>
>> all of which define
>> g-tx-fifo-size = <512 512 512 512 512 256 256>;
>>
>> Here the calculation holds:
>> 558 + 32 + 512 + 512 + 512 + 512 + 512 + 256 + 256 = 3662 < 4080
>>
>> My RPi4 uses one of these DTSIs, because my
>> /sys/kernel/debug/usb/fe980000.usb/params says:
>>
>> g_rx_fifo_size                : 558
>> g_np_tx_fifo_size             : 32
>> g_tx_fifo_size[0]             : 0
>> g_tx_fifo_size[1]             : 512
>> g_tx_fifo_size[2]             : 512
>> g_tx_fifo_size[3]             : 512
>> g_tx_fifo_size[4]             : 512
>> g_tx_fifo_size[5]             : 512
>> g_tx_fifo_size[6]             : 256
>> g_tx_fifo_size[7]             : 256
>>
>>
>> IIUC the tx_fifo values in bcm283x-rpi-usb-peripheral.dtsi and
>> bcm283x-rpi-usb-otg.dtsi files can be lowered to the values used and
>> tested (at least by me) in the RPi repo. But this is outside of my
>> knowledge, honestly I do not know what is the most appropriate
>> distribution of the remaining fifo space among the g_tx_fifo buffers.
>> Please can the RPi developers (Phil?) suggest a fix?
> 
> As author of the mainline bcm283x-rpi-usb-otg.dtsi i was trying to
> optimize the fifo sizes for EP 6 and 7. But i don't remember why. So my
> suggestion for a fix would be:
> 
> g-tx-fifo-size = <256 256 256 512 512 768 768>;
> 
> But i'm also unsure about the values.
> 

IIUC this code
https://github.com/torvalds/linux/blob/master/drivers/usb/dwc2/gadget.c#L4091
optimizes the FIFO assignment to endpoints. From that I would conclude
that correct values are specific for each use-case configuration of
endpoints. Maybe a varied selection (256, 512, 768) is more convenient
than just 256 and 512. I really do not know what use cases need what TX
fifo values.


BTW perhaps this comment
https://github.com/torvalds/linux/blob/master/drivers/usb/dwc2/gadget.c#L327
is a bit misleading since that code does not do the assignment, just
stores the size distribution to the DPTXFSIZN register, IIUC.

Best regards,

Pavel.

  reply	other threads:[~2021-05-28  8:59 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-13  7:18 [PATCH] ARM: dts: bcm283x: increase dwc2's RX FIFO size Pavel Hofman
2020-11-20 16:44 ` Nicolas Saenz Julienne
2021-05-26 17:12 ` Stefan Wahren
2021-05-27 13:17   ` Pavel Hofman
2021-05-27 13:47     ` Stefan Wahren
2021-05-28  8:59       ` Pavel Hofman [this message]
2021-08-06 13:03         ` Pavel Hofman
2021-08-06 14:08           ` Stefan Wahren
2021-08-06 14:46             ` Pavel Hofman
2021-08-06 15:57               ` Stefan Wahren
  -- strict thread matches above, loose matches on Subject: below --
2020-02-07 21:12 Pavel Hofman
2020-02-12 18:57 ` Nicolas Saenz Julienne

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a5b12552-1340-aa71-caca-fbef98b8b3e3@ivitera.com \
    --to=pavel.hofman@ivitera.com \
    --cc=devicetree@vger.kernel.org \
    --cc=hminas@synopsys.com \
    --cc=linux-rpi-kernel@lists.infradead.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=nsaenz@kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=stefan.wahren@i2se.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.