On 3/7/24 02:44, Thinh Nguyen wrote:
Hi,
On Tue, Feb 27, 2024, Pandey, Radhey Shyam wrote:
-----Original Message-----
From: Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx>
Sent: Saturday, February 24, 2024 4:38 AM
To: Pandey, Radhey Shyam <radhey.shyam.pandey@xxxxxxx>
Cc: gregkh@xxxxxxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; linux-
kernel@xxxxxxxxxxxxxxx; git (AMD-Xilinx) <git@xxxxxxx>
Subject: Re: [PATCH v2] usb: dwc3: core: enable CCI support for AMD-xilinx
DWC3 controller
On Fri, Feb 23, 2024, Thinh Nguyen wrote:
On Sat, Feb 24, 2024, Radhey Shyam Pandey wrote:
From: Piyush Mehta <piyush.mehta@xxxxxxx>
The GSBUSCFG0 register bits [31:16] are used to configure the cache type
settings of the descriptor and data write/read transfers (Cacheable,
Bufferable/ Posted). When CCI is enabled in the design, DWC3 core
GSBUSCFG0
cache bits must be updated to support CCI enabled transfers in USB.
Signed-off-by: Piyush Mehta <piyush.mehta@xxxxxxx>
Signed-off-by: Radhey Shyam Pandey
<radhey.shyam.pandey@xxxxxxx>
----
changes for v2:
Make GSBUSCFG0 configuration specific to AMD-xilinx platform.
Taken reference from existing commit ec5eb43813a4 ("usb: dwc3: core:
add support for realtek SoCs custom's global register start address")
Regarding that change from Realtek, it's a special case. I want to avoid
doing platform specific checks in the core.c if possible. Eventually, I
want to move that logic from Realtek to its glue driver.
BR,
Thinh
Thanks. As you suggested I tried "temporarily memory map and update this
register in your Xilinx glue driver. Its value should retain after soft reset".
Did ioremap for core register space once again in glue driver but it resulted
in below error:
dwc3 fe200000.usb: can't request region for resource [mem 0xfe200000-0xfe23ffff]
dwc3-xilinx ff9d0000.usb: error -EBUSY: failed to map DWC3 registers
So to avoid remapping, now get the struct dwc3 platform data handle in glue
driver and pass it to dwc3_readl/writel() like the below sequence. Is that fine?
If yes I will respin v3 with these changes and also do some more
sanity tests.
drivers/usb/dwc3/dwc3-xilinx.c
#include "io.h"
<snip>
ret = of_platform_populate(np, NULL, NULL, dev);
if (ret) {
dev_err(dev, "failed to register dwc3 core - %d\n", ret);
goto err_clk_put;
}
dwc3_np = of_get_compatible_child(np, "snps,dwc3");
priv_data->dwc3 = of_find_device_by_node(dwc3_np);
dwc = platform_get_drvdata(priv_data->dwc3);
if (of_dma_is_coherent(dev->of_node)) {
reg = dwc3_readl(dwc->regs , DWC3_GSBUSCFG0);
reg |= DWC3_GSBUSCFG0_DATRDREQINFO_MASK |
DWC3_GSBUSCFG0_DESRDREQINFO_MASK |
DWC3_GSBUSCFG0_DATWRREQINFO_MASK |
DWC3_GSBUSCFG0_DESWRREQINFO_MASK;
It's a bit odd to use "mask" as value instead of some defined
macros/values.
dwc3_writel(dwc->regs , DWC3_GSBUSCFG0, reg);
}
Perhaps it may be better to add a new interface for the core to interact
with the glue drivers. The core can use these callbacks to properly set
platform specific configuration at specified sequence of the controller
initialization. It will be better defined and more predictable than what
we have here. What do you think?
Not sure I fully understand what you mean by more predictable.
Are you asking for simple read/write interface to dwc3 core IP?
Do you want there any limitation for accesses to be able to control it?
I don't think we have any issue with your suggestion but it is unclear how
exactly it should look like.
Can you please sketch it?
Thanks,
Michal
