Re: [PATCH 1/2] usb: dwc2: Extract USB DWC2 register definitions

[Marek Vasut]

On 6/27/24 1:33 PM, Kongyang Liu wrote:
Marek Vasut <ma...@denx.de> 于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 0000000000..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 <seashell11234...@gmail.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/errno.h>
+#include <linux/io.h>
+#include <wait_bit.h>
+
+#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(&regs->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, &reg->diepmsk);
+       writel(DIEPMSK_INIT, &reg->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 !