[PATCH v2 7/8] usb: dwc2: Unify flush and reset logic with v4.20a support

[Marek Vasut]

On 12/30/24 4:30 AM, Junhui Liu via B4 Relay wrote:
> From: Kongyang Liu <seashell11234455 at gmail.com>
> 
> This patch merges flush and reset logic for both host and gadget code
> into a common set of functions, reducing duplication. It also adds support
> for the updated reset logic to compatible with core version since v4.20a.
> 
> This patch mainly refers to the patch in the kernel.
> link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=65dc2e725286106f99c6f6b78e3d9c52c15f3a9c
> 
> Signed-off-by: Kongyang Liu <seashell11234455 at gmail.com>
> Signed-off-by: Junhui Liu <liujh2818 at outlook.com>
> ---
>   drivers/usb/common/Makefile                |   2 +
>   drivers/usb/common/dwc2_core.c             | 103 +++++++++++++++++++++++++++++
>   drivers/usb/common/dwc2_core.h             |   4 ++
>   drivers/usb/gadget/dwc2_udc_otg.c          |  12 +---
>   drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c |   6 +-
>   drivers/usb/host/dwc2.c                    |  80 ++--------------------
>   6 files changed, 117 insertions(+), 90 deletions(-)
> 
> diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
> index 11cc4657a0f403b84b1b8336781e1893d9c7a8f1..73e5bc6d7fdca692276e119911b47db4bf03586a 100644
> --- a/drivers/usb/common/Makefile
> +++ b/drivers/usb/common/Makefile
> @@ -4,6 +4,8 @@
>   #
>   
>   obj-$(CONFIG_$(XPL_)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 0000000000000000000000000000000000000000..323326e05d7b0318d883d4a3b8186b812c42f9ca
> --- /dev/null
> +++ b/drivers/usb/common/dwc2_core.c
> @@ -0,0 +1,103 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2024, Kongyang Liu <seashell11234455 at gmail.com>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/errno.h>
> +#include <linux/io.h>
> +#include <wait_bit.h>
> +
> +#include "dwc2_core.h"
> +
> +int dwc2_core_reset(struct dwc2_core_regs *regs)
> +{
> +	u32 snpsid;
> +	int ret;
> +	bool host_mode = false;
> +
> +	if (!(readl(&regs->global_regs.gotgctl) & GOTGCTL_CONID_B) ||
> +	    (readl(&regs->global_regs.gusbcfg) & GUSBCFG_FORCEDEVMODE))
> +		host_mode = true;
> +
> +	/* Core Soft Reset */
> +	snpsid = readl(&regs->global_regs.gsnpsid);
> +	writel(GRSTCTL_CSFTRST, &regs->global_regs.grstctl);
> +	if (FIELD_GET(GSNPSID_VER_MASK, snpsid) < 0x420a) {
> +		ret = wait_for_bit_le32(&regs->global_regs.grstctl, GRSTCTL_CSFTRST,
> +					false, 1000, false);
> +		if (ret) {
> +			log_warning("%s: Waiting for GRSTCTL_CSFTRST timeout\n", __func__);
> +			return -EBUSY;

Return -ETIMEDOUT or even better, return ret; to propagate the return 
value of wait_for_bit_le32() as-is , which is really -ETIMEDOUT if the 
wait times out.

> +		}
> +	} else {
> +		ret = wait_for_bit_le32(&regs->global_regs.grstctl, GRSTCTL_CSFTRST_DONE,
> +					true, 1000, false);
> +		if (ret) {
> +			log_warning("%s: Waiting for GRSTCTL_CSFTRST_DONE timeout\n", __func__);
> +			return -EBUSY;

DTTO, return ret;

> +		}
> +		clrsetbits_le32(&regs->global_regs.grstctl, GRSTCTL_CSFTRST, GRSTCTL_CSFTRST_DONE);
> +	}
> +
> +	/* Wait for AHB master IDLE state. */
> +	ret = wait_for_bit_le32(&regs->global_regs.grstctl, GRSTCTL_AHBIDLE,
> +				true, 1000, false);
> +	if (ret) {
> +		log_warning("%s: Waiting for GRSTCTL_AHBIDLE timeout\n", __func__);
> +		return -EBUSY;

DTTO, return ret;

> +	}
> +
> +	if (host_mode) {
> +		ret = wait_for_bit_le32(&regs->global_regs.gintsts, GINTSTS_CURMODE_HOST,
> +					host_mode, 1000, false);
> +		if (ret) {
> +			log_warning("%s: Waiting for GINTSTS_CURMODE_HOST timeout\n", __func__);
> +			return -EBUSY;

DTTO, return ret;

> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +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);
> +	if (ret)
> +		log_warning("%s: Waiting for GRSTCTL_AHBIDLE timeout\n", __func__);


Should this function return an integer and an error code on failure ?

> +	writel(GRSTCTL_TXFFLSH | FIELD_PREP(GRSTCTL_TXFNUM_MASK, num), &regs->global_regs.grstctl);
> +
> +	ret = wait_for_bit_le32(&regs->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(&regs->global_regs.grstctl, GRSTCTL_AHBIDLE, true, 1000, false);
> +	if (ret)
> +		log_warning("%s: Waiting for GRSTCTL_AHBIDLE timeout\n", __func__);
> +
> +	writel(GRSTCTL_RXFFLSH, &regs->global_regs.grstctl);
> +
> +	ret = wait_for_bit_le32(&regs->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);

Shouldn't this delay be derived from the PHY clock frequency somehow ?
Are we sure 1us is always sufficient ?

> +}
[...]