Re: [PATCH 1/2] usb: dwc2: Extract USB DWC2 register definitions
On 6/27/24 1:33 PM, Kongyang Liu wrote:
Marek Vasut 于2024年6月23日周日 07:49写道：
On 5/22/24 4:22 PM, Kongyang Liu wrote:
Hi,
sorry for the late reply.
diff --git a/drivers/usb/common/dwc2_core.c b/drivers/usb/common/dwc2_core.c
new file mode 100644
index 00..2fa11fd59d
--- /dev/null
+++ b/drivers/usb/common/dwc2_core.c
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2024, Kongyang Liu
+ */
+
+#include
+#include
+#include
+#include
+
+#include "dwc2_core.h"
+
+void dwc2_flush_tx_fifo(struct dwc2_core_regs *regs, const int num)
+{
+ int ret;
+
+ log_debug("Flush Tx FIFO %d\n", num);
+
+ /* Wait for AHB master IDLE state */
+ ret = wait_for_bit_le32(>global_regs.grstctl, GRSTCTL_AHBIDLE,
true, 1000, false);
Just a quick design point, would it be possible to split this patch into
two, one which adds this .global_regs and changes the code accordingly,
and another which does the code refactoring/move ? That would make it
easier to review.
This patch only extracts the common parts of the host mode and gadget
mode, without any code refactoring. Could you please describe more clearly
how the patch should be split?
Sure, sorry for being unclear. There seems to be multiple types of
changes that are combined in this patch:
1) Structure reorganization:
- writel(DIEPMSK_INIT, >diepmsk);
+ writel(DIEPMSK_INIT, >device_regs.diepmsk);
2) Structure rename:
- reg = (struct dwc2_usbotg_reg *)pdata->regs_otg;
+ reg = (struct dwc2_core_regs *)pdata->regs_otg;
3) Conversion from (1 << n) to BIT(n)
4) Macro rename:
5) Moving code around between files:
#define DWC2_HWCFG4_IDDIG_FILT_EN (1 << 20)
#define GHWCFG4_IDDIG_FILT_EN BIT(20)
It would be good to have those as separate patches (one type of clean up
change per patch), otherwise it is really hard to review such a combined
patch.
Thanks !
Re: [PATCH 1/2] usb: dwc2: Extract USB DWC2 register definitions
Marek Vasut 于2024年6月23日周日 07:49写道：
>
> On 5/22/24 4:22 PM, Kongyang Liu wrote:
>
> Hi,
>
> sorry for the late reply.
>
> > diff --git a/drivers/usb/common/dwc2_core.c b/drivers/usb/common/dwc2_core.c
> > new file mode 100644
> > index 00..2fa11fd59d
> > --- /dev/null
> > +++ b/drivers/usb/common/dwc2_core.c
> > @@ -0,0 +1,53 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2024, Kongyang Liu
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include "dwc2_core.h"
> > +
> > +void dwc2_flush_tx_fifo(struct dwc2_core_regs *regs, const int num)
> > +{
> > + int ret;
> > +
> > + log_debug("Flush Tx FIFO %d\n", num);
> > +
> > + /* Wait for AHB master IDLE state */
> > + ret = wait_for_bit_le32(>global_regs.grstctl, GRSTCTL_AHBIDLE,
> > true, 1000, false);
>
> Just a quick design point, would it be possible to split this patch into
> two, one which adds this .global_regs and changes the code accordingly,
> and another which does the code refactoring/move ? That would make it
> easier to review.
This patch only extracts the common parts of the host mode and gadget
mode, without any code refactoring. Could you please describe more clearly
how the patch should be split?
Best regards
Kongyang Liu
Re: [PATCH 1/2] usb: dwc2: Extract USB DWC2 register definitions
On 5/22/24 4:22 PM, Kongyang Liu wrote:
Hi,
sorry for the late reply.
diff --git a/drivers/usb/common/dwc2_core.c b/drivers/usb/common/dwc2_core.c
new file mode 100644
index 00..2fa11fd59d
--- /dev/null
+++ b/drivers/usb/common/dwc2_core.c
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2024, Kongyang Liu
+ */
+
+#include
+#include
+#include
+#include
+
+#include "dwc2_core.h"
+
+void dwc2_flush_tx_fifo(struct dwc2_core_regs *regs, const int num)
+{
+ int ret;
+
+ log_debug("Flush Tx FIFO %d\n", num);
+
+ /* Wait for AHB master IDLE state */
+ ret = wait_for_bit_le32(>global_regs.grstctl, GRSTCTL_AHBIDLE,
true, 1000, false);
Just a quick design point, would it be possible to split this patch into
two, one which adds this .global_regs and changes the code accordingly,
and another which does the code refactoring/move ? That would make it
easier to review.
[PATCH 1/2] usb: dwc2: Extract USB DWC2 register definitions
Extract the register definitions and their bit definitions from the USB
DWC2 driver host and device into a common file.
Signed-off-by: Kongyang Liu
---
drivers/usb/common/Makefile| 2 +
drivers/usb/common/dwc2_core.c | 53 ++
drivers/usb/common/dwc2_core.h | 556
drivers/usb/gadget/dwc2_udc_otg.c | 124 ++--
drivers/usb/gadget/dwc2_udc_otg_regs.h | 247 +--
drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 306 -
drivers/usb/host/dwc2.c| 362 +-
drivers/usb/host/dwc2.h| 736 -
8 files changed, 988 insertions(+), 1398 deletions(-)
create mode 100644 drivers/usb/common/dwc2_core.c
create mode 100644 drivers/usb/common/dwc2_core.h
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 2e9353b76a..708f13c52c 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -4,6 +4,8 @@
#
obj-$(CONFIG_$(SPL_)DM_USB) += common.o
+obj-$(CONFIG_USB_DWC2) += dwc2_core.o
+obj-$(CONFIG_USB_GADGET_DWC2_OTG) += dwc2_core.o
obj-$(CONFIG_USB_ISP1760) += usb_urb.o
obj-$(CONFIG_USB_MUSB_HOST) += usb_urb.o
obj-$(CONFIG_USB_MUSB_GADGET) += usb_urb.o
diff --git a/drivers/usb/common/dwc2_core.c b/drivers/usb/common/dwc2_core.c
new file mode 100644
index 00..2fa11fd59d
--- /dev/null
+++ b/drivers/usb/common/dwc2_core.c
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2024, Kongyang Liu
+ */
+
+#include
+#include
+#include
+#include
+
+#include "dwc2_core.h"
+
+void dwc2_flush_tx_fifo(struct dwc2_core_regs *regs, const int num)
+{
+ int ret;
+
+ log_debug("Flush Tx FIFO %d\n", num);
+
+ /* Wait for AHB master IDLE state */
+ ret = wait_for_bit_le32(>global_regs.grstctl, GRSTCTL_AHBIDLE,
true, 1000, false);
+ if (ret)
+ log_warning("%s: Waiting for GRSTCTL_AHBIDLE timeout\n",
__func__);
+
+ writel(GRSTCTL_TXFFLSH | FIELD_PREP(GRSTCTL_TXFNUM_MASK, num),
>global_regs.grstctl);
+
+ ret = wait_for_bit_le32(>global_regs.grstctl, GRSTCTL_TXFFLSH,
false, 1000, false);
+ if (ret)
+ log_warning("%s: Waiting for GRSTCTL_TXFFLSH timeout\n",
__func__);
+
+ /* Wait for 3 PHY Clocks */
+ udelay(1);
+}
+
+void dwc2_flush_rx_fifo(struct dwc2_core_regs *regs)
+{
+ int ret;
+
+ log_debug("Flush Rx FIFO\n");
+
+ /* Wait for AHB master IDLE state */
+ ret = wait_for_bit_le32(>global_regs.grstctl, GRSTCTL_AHBIDLE,
true, 1000, false);
+ if (ret)
+ log_warning("%s: Waiting for GRSTCTL_AHBIDLE timeout\n",
__func__);
+
+ writel(GRSTCTL_RXFFLSH, >global_regs.grstctl);
+
+ ret = wait_for_bit_le32(>global_regs.grstctl, GRSTCTL_RXFFLSH,
false, 1000, false);
+ if (ret)
+ log_warning("%s: Waiting for GRSTCTL_RXFFLSH timeout\n",
__func__);
+
+ /* Wait for 3 PHY Clocks */
+ udelay(1);
+}
diff --git a/drivers/usb/common/dwc2_core.h b/drivers/usb/common/dwc2_core.h
new file mode 100644
index 00..8303153446
--- /dev/null
+++ b/drivers/usb/common/dwc2_core.h
@@ -0,0 +1,556 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2024, Kongyang Liu
+ *
+ */
+
+#ifndef __DWC2_CORE_H_
+#define __DWC2_CORE_H_
+
+#include
+
+struct dwc2_global_regs {
+ u32 gotgctl;
+ u32 gotgint;
+ u32 gahbcfg;
+ u32 gusbcfg;
+ u32 grstctl;
+ u32 gintsts;
+ u32 gintmsk;
+ u32 grxstsr;
+ u32 grxstsp;
+ u32 grxfsiz;
+ u32 gnptxfsiz;
+ u32 gnptxsts;
+ u32 gi2cctl;
+ u32 gpvndctl;
+ u32 ggpio;
+ u32 guid;
+ u32 gsnpsid;
+ u32 ghwcfg1;
+ u32 ghwcfg2;
+ u32 ghwcfg3;
+ u32 ghwcfg4;
+ u32 glpmcfg;
+ u32 gpwrdn;
+ u32 gdfifocfg;
+ u32 gadpctl;
+ u32 grefclk;
+ u32 gintmsk2;
+ u32 gintsts2;
+ u8 _pad_from_0x70_to_0x100[0x100 - 0x70];
+ u32 hptxfsiz;
+ u32 dptxfsizn[15];
+ u8 _pad_from_0x140_to_0x400[0x400 - 0x140];
+};
+
+struct dwc2_hc_regs {
+ u32 hcchar;
+ u32 hcsplt;
+ u32 hcint;
+ u32 hcintmsk;
+ u32 hctsiz;
+ u32 hcdma;
+ u32 reserved;
+ u32 hcdmab;
+};
+
+struct dwc2_host_regs {
+ u32 hcfg;
+ u32 hfir;
+ u32 hfnum;
+ u32 _pad_0x40c;
+ u32 hptxsts;
+ u32 haint;
+ u32 haintmsk;
+ u32 hflbaddr;
+ u8 _pad_from_0x420_to_0x440[0x440 - 0x420];
+ u32 hprt0;
+ u8 _pad_from_0x444_to_0x500[0x500 - 0x444];
+ struct dwc2_hc_regs hc[16];
+ u8 _pad_from_0x700_to_0x800[0x800 - 0x700];
+};
+
+struct dwc2_dev_in_endp {
+ u32 diepctl;
+ u32 reserved0;
+ u32 diepint;
+ u32 reserved1;
+ u32 dieptsiz;
+ u32 diepdma;
+ u32 reserved2;
+ u32 diepdmab;
+};
+
+struct dwc2_dev_out_endp {
+ u32 doepctl;
+ u32
