[PATCH] usb: cdnsp: add support for eUSB2v2 port

[PATCH] usb: cdnsp: add support for eUSB2v2 port

[Pawel Laszczak via B4 Relay]

From: Pawel Laszczak <pawell@cadence.com>

The Cadence CDNSP controller optionally supports eUSB2 (embedded USB2)
port. While this port type operates logically like high-speed USB 2.0,
it utilizes a different physical layer signaling.

This patch:
- Extends the port detection logic to recognize the eUSB2 protocol.
- Tracks the eUSB2 port offset in the cdnsp_device structure.
- Ensures that eUSB2 ports are correctly handled during Link State
  transitions, specifically forcing L0 when LPM is capable, similar
  to standard USB 2.0 ports.

Signed-off-by: Pawel Laszczak <pawell@cadence.com>
---
 drivers/usb/cdns3/cdnsp-gadget.c | 49 ++++++++++++++++++---------
 drivers/usb/cdns3/cdnsp-gadget.h |  1 +
 drivers/usb/cdns3/cdnsp-mem.c    | 73 +++++++++++++++++++++++++++-------------
 drivers/usb/cdns3/cdnsp-ring.c   |  9 +++--
 4 files changed, 90 insertions(+), 42 deletions(-)

diff --git a/drivers/usb/cdns3/cdnsp-gadget.c b/drivers/usb/cdns3/cdnsp-gadget.c
index 6b3815f8a6e5..2c71c77e6ec3 100644
--- a/drivers/usb/cdns3/cdnsp-gadget.c
+++ b/drivers/usb/cdns3/cdnsp-gadget.c
@@ -124,20 +124,28 @@ void cdnsp_set_link_state(struct cdnsp_device *pdev,
 }
 
 static void cdnsp_disable_port(struct cdnsp_device *pdev,
-			       __le32 __iomem *port_regs)
+			       struct cdnsp_port *port)
 {
-	u32 temp = cdnsp_port_state_to_neutral(readl(port_regs));
+	u32 temp;
+
+	if (!port->exist)
+		return;
 
-	writel(temp | PORT_PED, port_regs);
+	temp = cdnsp_port_state_to_neutral(readl(&port->regs->portsc));
+	writel(temp | PORT_PED, &port->regs->portsc);
 }
 
 static void cdnsp_clear_port_change_bit(struct cdnsp_device *pdev,
-					__le32 __iomem *port_regs)
+					struct cdnsp_port *port)
 {
-	u32 portsc = readl(port_regs);
+	u32 portsc;
+
+	if (!port->exist)
+		return;
 
+	portsc = readl(&port->regs->portsc);
 	writel(cdnsp_port_state_to_neutral(portsc) |
-	       (portsc & PORT_CHANGE_BITS), port_regs);
+	       (portsc & PORT_CHANGE_BITS), &port->regs->portsc);
 }
 
 static void cdnsp_set_apb_timeout_value(struct cdnsp_device *pdev)
@@ -944,7 +952,7 @@ void cdnsp_set_usb2_hardware_lpm(struct cdnsp_device *pdev,
 				 struct usb_request *req,
 				 int enable)
 {
-	if (pdev->active_port != &pdev->usb2_port || !pdev->gadget.lpm_capable)
+	if (pdev->active_port == &pdev->usb3_port || !pdev->gadget.lpm_capable)
 		return;
 
 	trace_cdnsp_lpm(enable);
@@ -1310,20 +1318,26 @@ static int cdnsp_run(struct cdnsp_device *pdev,
 		break;
 	}
 
-	if (speed >= USB_SPEED_SUPER) {
+	if (pdev->usb3_port.exist && speed >= USB_SPEED_SUPER) {
 		writel(temp, &pdev->port3x_regs->mode_addr);
 		cdnsp_set_link_state(pdev, &pdev->usb3_port.regs->portsc,
 				     XDEV_RXDETECT);
 	} else {
-		cdnsp_disable_port(pdev, &pdev->usb3_port.regs->portsc);
+		cdnsp_disable_port(pdev, &pdev->usb3_port);
 	}
 
-	cdnsp_set_link_state(pdev, &pdev->usb2_port.regs->portsc,
-			     XDEV_RXDETECT);
+	if (pdev->usb2_port.exist) {
+		cdnsp_set_link_state(pdev, &pdev->usb2_port.regs->portsc,
+				     XDEV_RXDETECT);
+		writel(PORT_REG6_L1_L0_HW_EN | fs_speed, &pdev->port20_regs->port_reg6);
+	}
+
+	if (pdev->eusb_port.exist)
+		cdnsp_set_link_state(pdev, &pdev->eusb_port.regs->portsc,
+				     XDEV_RXDETECT);
 
 	cdnsp_gadget_ep0_desc.wMaxPacketSize = cpu_to_le16(512);
 
-	writel(PORT_REG6_L1_L0_HW_EN | fs_speed, &pdev->port20_regs->port_reg6);
 
 	ret = cdnsp_start(pdev);
 	if (ret) {
@@ -1469,8 +1483,10 @@ static void cdnsp_stop(struct cdnsp_device *pdev)
 			cdnsp_ep_dequeue(&pdev->eps[0], req);
 	}
 
-	cdnsp_disable_port(pdev, &pdev->usb2_port.regs->portsc);
-	cdnsp_disable_port(pdev, &pdev->usb3_port.regs->portsc);
+	cdnsp_disable_port(pdev, &pdev->usb2_port);
+	cdnsp_disable_port(pdev, &pdev->usb3_port);
+	cdnsp_disable_port(pdev, &pdev->eusb_port);
+
 	cdnsp_disable_slot(pdev);
 	cdnsp_halt(pdev);
 
@@ -1479,8 +1495,9 @@ static void cdnsp_stop(struct cdnsp_device *pdev)
 	temp = readl(&pdev->ir_set->irq_pending);
 	writel(IMAN_IE_CLEAR(temp), &pdev->ir_set->irq_pending);
 
-	cdnsp_clear_port_change_bit(pdev, &pdev->usb2_port.regs->portsc);
-	cdnsp_clear_port_change_bit(pdev, &pdev->usb3_port.regs->portsc);
+	cdnsp_clear_port_change_bit(pdev, &pdev->usb2_port);
+	cdnsp_clear_port_change_bit(pdev, &pdev->eusb_port);
+	cdnsp_clear_port_change_bit(pdev, &pdev->usb3_port);
 
 	/* Clear interrupt line */
 	temp = readl(&pdev->ir_set->irq_pending);
diff --git a/drivers/usb/cdns3/cdnsp-gadget.h b/drivers/usb/cdns3/cdnsp-gadget.h
index a91cca509db0..c44bca348a41 100644
--- a/drivers/usb/cdns3/cdnsp-gadget.h
+++ b/drivers/usb/cdns3/cdnsp-gadget.h
@@ -1474,6 +1474,7 @@ struct cdnsp_device {
 	unsigned int link_state;
 
 	struct cdnsp_port usb2_port;
+	struct cdnsp_port eusb_port;
 	struct cdnsp_port usb3_port;
 	struct cdnsp_port *active_port;
 	u16 test_mode;
diff --git a/drivers/usb/cdns3/cdnsp-mem.c b/drivers/usb/cdns3/cdnsp-mem.c
index a2a1b21f2ef8..932071818d1e 100644
--- a/drivers/usb/cdns3/cdnsp-mem.c
+++ b/drivers/usb/cdns3/cdnsp-mem.c
@@ -1088,11 +1088,9 @@ void cdnsp_mem_cleanup(struct cdnsp_device *pdev)
 			  pdev->dcbaa, pdev->dcbaa->dma);
 
 	pdev->dcbaa = NULL;
-
-	pdev->usb2_port.exist = 0;
-	pdev->usb3_port.exist = 0;
-	pdev->usb2_port.port_num = 0;
-	pdev->usb3_port.port_num = 0;
+	memset(&pdev->usb2_port, 0, sizeof(struct cdnsp_port));
+	memset(&pdev->eusb_port, 0, sizeof(struct cdnsp_port));
+	memset(&pdev->usb3_port, 0, sizeof(struct cdnsp_port));
 	pdev->active_port = NULL;
 }
 
@@ -1133,6 +1131,18 @@ static void cdnsp_add_in_port(struct cdnsp_device *pdev,
 	port_offset = CDNSP_EXT_PORT_OFF(temp);
 	port_count = CDNSP_EXT_PORT_COUNT(temp);
 
+	if (port == &pdev->eusb_port) {
+		/*
+		 * If controller has usb2 + eusb port then eusb is as
+		 * second port
+		 */
+		if (port_count == 2)
+			port_offset++;
+
+		if (port_count == 1 && pdev->usb2_port.exist)
+			return;
+	}
+
 	trace_cdnsp_port_info(addr, port_offset, port_count, port->maj_rev);
 
 	port->port_num = port_offset;
@@ -1152,13 +1162,10 @@ static int cdnsp_setup_port_arrays(struct cdnsp_device *pdev)
 	base = &pdev->cap_regs->hc_capbase;
 	offset = cdnsp_find_next_ext_cap(base, 0,
 					 EXT_CAP_CFG_DEV_20PORT_CAP_ID);
-	pdev->port20_regs = base + offset;
-
-	offset = cdnsp_find_next_ext_cap(base, 0, D_XEC_CFG_3XPORT_CAP);
-	pdev->port3x_regs =  base + offset;
+	if (offset)
+		pdev->port20_regs = base + offset;
 
 	offset = 0;
-	base = &pdev->cap_regs->hc_capbase;
 
 	/* Driver expects max 2 extended protocol capability. */
 	for (i = 0; i < 2; i++) {
@@ -1173,26 +1180,46 @@ static int cdnsp_setup_port_arrays(struct cdnsp_device *pdev)
 			cdnsp_add_in_port(pdev, &pdev->usb3_port,
 					  base + offset);
 
-		if (CDNSP_EXT_PORT_MAJOR(temp) == 0x02 &&
-		    !pdev->usb2_port.port_num)
-			cdnsp_add_in_port(pdev, &pdev->usb2_port,
-					  base + offset);
+		if (CDNSP_EXT_PORT_MAJOR(temp) == 0x02) {
+			if (!pdev->usb2_port.port_num && pdev->port20_regs)
+				cdnsp_add_in_port(pdev, &pdev->usb2_port,
+						  base + offset);
+
+			if (!pdev->eusb_port.port_num)
+				cdnsp_add_in_port(pdev, &pdev->eusb_port,
+						  base + offset);
+		}
 	}
 
-	if (!pdev->usb2_port.exist || !pdev->usb3_port.exist) {
-		dev_err(pdev->dev, "Error: Only one port detected\n");
+	if (!pdev->usb2_port.exist && !pdev->eusb_port.exist &&
+	    !pdev->usb3_port.exist) {
+		dev_err(pdev->dev, "Error: No port detected\n");
 		return -ENODEV;
 	}
 
-	trace_cdnsp_init("Found USB 2.0 ports and  USB 3.0 ports.");
+	if (pdev->usb2_port.exist) {
+		pdev->usb2_port.regs = (struct cdnsp_port_regs __iomem *)
+				       (&pdev->op_regs->port_reg_base + NUM_PORT_REGS *
+					(pdev->usb2_port.port_num - 1));
+		trace_cdnsp_init("Found USB 2.0 port.");
+	}
 
-	pdev->usb2_port.regs = (struct cdnsp_port_regs __iomem *)
-			       (&pdev->op_regs->port_reg_base + NUM_PORT_REGS *
-				(pdev->usb2_port.port_num - 1));
+	if (pdev->eusb_port.exist) {
+		pdev->eusb_port.regs = (struct cdnsp_port_regs __iomem *)
+				       (&pdev->op_regs->port_reg_base + NUM_PORT_REGS *
+					(pdev->eusb_port.port_num - 1));
+		trace_cdnsp_init("Found eUSB 2.0 port.");
+	}
+
+	if (pdev->usb3_port.exist) {
+		offset = cdnsp_find_next_ext_cap(base, 0, D_XEC_CFG_3XPORT_CAP);
+		pdev->port3x_regs =  base + offset;
 
-	pdev->usb3_port.regs = (struct cdnsp_port_regs __iomem *)
-			       (&pdev->op_regs->port_reg_base + NUM_PORT_REGS *
-				(pdev->usb3_port.port_num - 1));
+		pdev->usb3_port.regs = (struct cdnsp_port_regs __iomem *)
+				       (&pdev->op_regs->port_reg_base + NUM_PORT_REGS *
+					(pdev->usb3_port.port_num - 1));
+		trace_cdnsp_init("Found  USB 3.x port.");
+	}
 
 	return 0;
 }
diff --git a/drivers/usb/cdns3/cdnsp-ring.c b/drivers/usb/cdns3/cdnsp-ring.c
index 0758f171f73e..715658c981ff 100644
--- a/drivers/usb/cdns3/cdnsp-ring.c
+++ b/drivers/usb/cdns3/cdnsp-ring.c
@@ -259,7 +259,7 @@ static bool cdnsp_room_on_ring(struct cdnsp_device *pdev,
  */
 static void cdnsp_force_l0_go(struct cdnsp_device *pdev)
 {
-	if (pdev->active_port == &pdev->usb2_port && pdev->gadget.lpm_capable)
+	if (pdev->active_port != &pdev->usb3_port && pdev->gadget.lpm_capable)
 		cdnsp_set_link_state(pdev, &pdev->active_port->regs->portsc, XDEV_U0);
 }
 
@@ -763,6 +763,8 @@ static int cdnsp_update_port_id(struct cdnsp_device *pdev, u32 port_id)
 
 	if (port_id == pdev->usb2_port.port_num) {
 		port = &pdev->usb2_port;
+	} else if (port_id == pdev->eusb_port.port_num) {
+		port = &pdev->eusb_port;
 	} else if (port_id == pdev->usb3_port.port_num) {
 		port  = &pdev->usb3_port;
 	} else {
@@ -779,7 +781,8 @@ static int cdnsp_update_port_id(struct cdnsp_device *pdev, u32 port_id)
 		cdnsp_enable_slot(pdev);
 	}
 
-	if (port_id == pdev->usb2_port.port_num)
+	if ((pdev->usb2_port.exist && port_id == pdev->usb2_port.port_num) ||
+	    (pdev->eusb_port.exist && port_id == pdev->eusb_port.port_num))
 		cdnsp_set_usb2_hardware_lpm(pdev, NULL, 1);
 	else
 		writel(PORT_U1_TIMEOUT(1) | PORT_U2_TIMEOUT(1),
@@ -808,7 +811,7 @@ static void cdnsp_handle_port_status(struct cdnsp_device *pdev,
 
 	port_regs = pdev->active_port->regs;
 
-	if (port_id == pdev->usb2_port.port_num)
+	if (port_id == pdev->usb2_port.port_num || port_id == pdev->eusb_port.port_num)
 		port2 = true;
 
 new_event:

---
base-commit: 1c7cc4904160c6fc6377564140062d68a3dc93a0
change-id: 20260417-eusb2v2_upstream-80c5b29a7bba

Best regards,
--  
Pawel Laszczak <pawell@cadence.com>

Re: [PATCH] usb: cdnsp: add support for eUSB2v2 port

[Peter Chen (CIX)]

On 26-04-17 10:37:31, Pawel Laszczak via B4 Relay wrote:
> From: Pawel Laszczak <pawell@cadence.com>
> 
> The Cadence CDNSP controller optionally supports eUSB2 (embedded USB2)
> port. While this port type operates logically like high-speed USB 2.0,
> it utilizes a different physical layer signaling.
> 
> This patch:
> - Extends the port detection logic to recognize the eUSB2 protocol.
> - Tracks the eUSB2 port offset in the cdnsp_device structure.
> - Ensures that eUSB2 ports are correctly handled during Link State
>   transitions, specifically forcing L0 when LPM is capable, similar
>   to standard USB 2.0 ports.
> 
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>

Pawel, I would like double confirm if you have tested current USB2 and
USB3 device mode, basically, I think you did it.

> ---
>  drivers/usb/cdns3/cdnsp-gadget.c | 49 ++++++++++++++++++---------
>  drivers/usb/cdns3/cdnsp-gadget.h |  1 +
>  drivers/usb/cdns3/cdnsp-mem.c    | 73 +++++++++++++++++++++++++++-------------
>  drivers/usb/cdns3/cdnsp-ring.c   |  9 +++--
>  4 files changed, 90 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/usb/cdns3/cdnsp-gadget.c b/drivers/usb/cdns3/cdnsp-gadget.c
> index 6b3815f8a6e5..2c71c77e6ec3 100644
> --- a/drivers/usb/cdns3/cdnsp-gadget.c
> +++ b/drivers/usb/cdns3/cdnsp-gadget.c
> @@ -124,20 +124,28 @@ void cdnsp_set_link_state(struct cdnsp_device *pdev,
>  }
>  
>  static void cdnsp_disable_port(struct cdnsp_device *pdev,
> -			       __le32 __iomem *port_regs)
> +			       struct cdnsp_port *port)
>  {
> -	u32 temp = cdnsp_port_state_to_neutral(readl(port_regs));
> +	u32 temp;
> +
> +	if (!port->exist)
> +		return;
>  
> -	writel(temp | PORT_PED, port_regs);
> +	temp = cdnsp_port_state_to_neutral(readl(&port->regs->portsc));
> +	writel(temp | PORT_PED, &port->regs->portsc);
>  }

Why above changes are added, is it related to this change?

>  
>  static void cdnsp_clear_port_change_bit(struct cdnsp_device *pdev,
> -					__le32 __iomem *port_regs)
> +					struct cdnsp_port *port)
>  {
> -	u32 portsc = readl(port_regs);
> +	u32 portsc;
> +
> +	if (!port->exist)
> +		return;
>  
> +	portsc = readl(&port->regs->portsc);
>  	writel(cdnsp_port_state_to_neutral(portsc) |
> -	       (portsc & PORT_CHANGE_BITS), port_regs);
> +	       (portsc & PORT_CHANGE_BITS), &port->regs->portsc);
>  }

ditto

>  
>  static void cdnsp_set_apb_timeout_value(struct cdnsp_device *pdev)
> @@ -944,7 +952,7 @@ void cdnsp_set_usb2_hardware_lpm(struct cdnsp_device *pdev,
>  				 struct usb_request *req,
>  				 int enable)
>  {
> -	if (pdev->active_port != &pdev->usb2_port || !pdev->gadget.lpm_capable)
> +	if (pdev->active_port == &pdev->usb3_port || !pdev->gadget.lpm_capable)
>  		return;
>  
>  	trace_cdnsp_lpm(enable);
> @@ -1310,20 +1318,26 @@ static int cdnsp_run(struct cdnsp_device *pdev,
>  		break;
>  	}
>  
> -	if (speed >= USB_SPEED_SUPER) {
> +	if (pdev->usb3_port.exist && speed >= USB_SPEED_SUPER) {
>  		writel(temp, &pdev->port3x_regs->mode_addr);
>  		cdnsp_set_link_state(pdev, &pdev->usb3_port.regs->portsc,
>  				     XDEV_RXDETECT);
>  	} else {
> -		cdnsp_disable_port(pdev, &pdev->usb3_port.regs->portsc);
> +		cdnsp_disable_port(pdev, &pdev->usb3_port);
>  	}
>  
> -	cdnsp_set_link_state(pdev, &pdev->usb2_port.regs->portsc,
> -			     XDEV_RXDETECT);
> +	if (pdev->usb2_port.exist) {
> +		cdnsp_set_link_state(pdev, &pdev->usb2_port.regs->portsc,
> +				     XDEV_RXDETECT);
> +		writel(PORT_REG6_L1_L0_HW_EN | fs_speed, &pdev->port20_regs->port_reg6);
> +	}
> +
> +	if (pdev->eusb_port.exist)
> +		cdnsp_set_link_state(pdev, &pdev->eusb_port.regs->portsc,
> +				     XDEV_RXDETECT);
>  
>  	cdnsp_gadget_ep0_desc.wMaxPacketSize = cpu_to_le16(512);
>  
> -	writel(PORT_REG6_L1_L0_HW_EN | fs_speed, &pdev->port20_regs->port_reg6);
>  
>  	ret = cdnsp_start(pdev);
>  	if (ret) {
> @@ -1469,8 +1483,10 @@ static void cdnsp_stop(struct cdnsp_device *pdev)
>  			cdnsp_ep_dequeue(&pdev->eps[0], req);
>  	}
>  
> -	cdnsp_disable_port(pdev, &pdev->usb2_port.regs->portsc);
> -	cdnsp_disable_port(pdev, &pdev->usb3_port.regs->portsc);
> +	cdnsp_disable_port(pdev, &pdev->usb2_port);
> +	cdnsp_disable_port(pdev, &pdev->usb3_port);
> +	cdnsp_disable_port(pdev, &pdev->eusb_port);
> +
>  	cdnsp_disable_slot(pdev);
>  	cdnsp_halt(pdev);
>  
> @@ -1479,8 +1495,9 @@ static void cdnsp_stop(struct cdnsp_device *pdev)
>  	temp = readl(&pdev->ir_set->irq_pending);
>  	writel(IMAN_IE_CLEAR(temp), &pdev->ir_set->irq_pending);
>  
> -	cdnsp_clear_port_change_bit(pdev, &pdev->usb2_port.regs->portsc);
> -	cdnsp_clear_port_change_bit(pdev, &pdev->usb3_port.regs->portsc);
> +	cdnsp_clear_port_change_bit(pdev, &pdev->usb2_port);
> +	cdnsp_clear_port_change_bit(pdev, &pdev->eusb_port);
> +	cdnsp_clear_port_change_bit(pdev, &pdev->usb3_port);
>  
>  	/* Clear interrupt line */
>  	temp = readl(&pdev->ir_set->irq_pending);
> diff --git a/drivers/usb/cdns3/cdnsp-gadget.h b/drivers/usb/cdns3/cdnsp-gadget.h
> index a91cca509db0..c44bca348a41 100644
> --- a/drivers/usb/cdns3/cdnsp-gadget.h
> +++ b/drivers/usb/cdns3/cdnsp-gadget.h
> @@ -1474,6 +1474,7 @@ struct cdnsp_device {
>  	unsigned int link_state;
>  
>  	struct cdnsp_port usb2_port;
> +	struct cdnsp_port eusb_port;
>  	struct cdnsp_port usb3_port;
>  	struct cdnsp_port *active_port;
>  	u16 test_mode;
> diff --git a/drivers/usb/cdns3/cdnsp-mem.c b/drivers/usb/cdns3/cdnsp-mem.c
> index a2a1b21f2ef8..932071818d1e 100644
> --- a/drivers/usb/cdns3/cdnsp-mem.c
> +++ b/drivers/usb/cdns3/cdnsp-mem.c
> @@ -1088,11 +1088,9 @@ void cdnsp_mem_cleanup(struct cdnsp_device *pdev)
>  			  pdev->dcbaa, pdev->dcbaa->dma);
>  
>  	pdev->dcbaa = NULL;
> -
> -	pdev->usb2_port.exist = 0;
> -	pdev->usb3_port.exist = 0;
> -	pdev->usb2_port.port_num = 0;
> -	pdev->usb3_port.port_num = 0;
> +	memset(&pdev->usb2_port, 0, sizeof(struct cdnsp_port));
> +	memset(&pdev->eusb_port, 0, sizeof(struct cdnsp_port));
> +	memset(&pdev->usb3_port, 0, sizeof(struct cdnsp_port));
>  	pdev->active_port = NULL;
>  }
>  
> @@ -1133,6 +1131,18 @@ static void cdnsp_add_in_port(struct cdnsp_device *pdev,
>  	port_offset = CDNSP_EXT_PORT_OFF(temp);
>  	port_count = CDNSP_EXT_PORT_COUNT(temp);
>  
> +	if (port == &pdev->eusb_port) {
> +		/*
> +		 * If controller has usb2 + eusb port then eusb is as
> +		 * second port
> +		 */

What kinds of topology like below usb2 + eusb?

> +		if (port_count == 2)
> +			port_offset++;
> +
> +		if (port_count == 1 && pdev->usb2_port.exist)
> +			return;
> +	}
> +
>  	trace_cdnsp_port_info(addr, port_offset, port_count, port->maj_rev);
>  
>  	port->port_num = port_offset;
> @@ -1152,13 +1162,10 @@ static int cdnsp_setup_port_arrays(struct cdnsp_device *pdev)
>  	base = &pdev->cap_regs->hc_capbase;
>  	offset = cdnsp_find_next_ext_cap(base, 0,
>  					 EXT_CAP_CFG_DEV_20PORT_CAP_ID);
> -	pdev->port20_regs = base + offset;
> -
> -	offset = cdnsp_find_next_ext_cap(base, 0, D_XEC_CFG_3XPORT_CAP);
> -	pdev->port3x_regs =  base + offset;
> +	if (offset)
> +		pdev->port20_regs = base + offset;
>  
>  	offset = 0;
> -	base = &pdev->cap_regs->hc_capbase;
>  
>  	/* Driver expects max 2 extended protocol capability. */
>  	for (i = 0; i < 2; i++) {
> @@ -1173,26 +1180,46 @@ static int cdnsp_setup_port_arrays(struct cdnsp_device *pdev)
>  			cdnsp_add_in_port(pdev, &pdev->usb3_port,
>  					  base + offset);
>  
> -		if (CDNSP_EXT_PORT_MAJOR(temp) == 0x02 &&
> -		    !pdev->usb2_port.port_num)
> -			cdnsp_add_in_port(pdev, &pdev->usb2_port,
> -					  base + offset);
> +		if (CDNSP_EXT_PORT_MAJOR(temp) == 0x02) {
> +			if (!pdev->usb2_port.port_num && pdev->port20_regs)

Why "&& pdev->port20_regs" is added?

> +				cdnsp_add_in_port(pdev, &pdev->usb2_port,
> +						  base + offset);
> +
> +			if (!pdev->eusb_port.port_num)
> +				cdnsp_add_in_port(pdev, &pdev->eusb_port,
> +						  base + offset);
> +		}
>  	}
>  
> -	if (!pdev->usb2_port.exist || !pdev->usb3_port.exist) {
> -		dev_err(pdev->dev, "Error: Only one port detected\n");
> +	if (!pdev->usb2_port.exist && !pdev->eusb_port.exist &&
> +	    !pdev->usb3_port.exist) {
> +		dev_err(pdev->dev, "Error: No port detected\n");
>  		return -ENODEV;
>  	}
>  
> -	trace_cdnsp_init("Found USB 2.0 ports and  USB 3.0 ports.");
> +	if (pdev->usb2_port.exist) {
> +		pdev->usb2_port.regs = (struct cdnsp_port_regs __iomem *)
> +				       (&pdev->op_regs->port_reg_base + NUM_PORT_REGS *
> +					(pdev->usb2_port.port_num - 1));
> +		trace_cdnsp_init("Found USB 2.0 port.");
> +	}
>  
> -	pdev->usb2_port.regs = (struct cdnsp_port_regs __iomem *)
> -			       (&pdev->op_regs->port_reg_base + NUM_PORT_REGS *
> -				(pdev->usb2_port.port_num - 1));
> +	if (pdev->eusb_port.exist) {
> +		pdev->eusb_port.regs = (struct cdnsp_port_regs __iomem *)
> +				       (&pdev->op_regs->port_reg_base + NUM_PORT_REGS *
> +					(pdev->eusb_port.port_num - 1));
> +		trace_cdnsp_init("Found eUSB 2.0 port.");
> +	}
> +
> +	if (pdev->usb3_port.exist) {
> +		offset = cdnsp_find_next_ext_cap(base, 0, D_XEC_CFG_3XPORT_CAP);
> +		pdev->port3x_regs =  base + offset;
>  
> -	pdev->usb3_port.regs = (struct cdnsp_port_regs __iomem *)
> -			       (&pdev->op_regs->port_reg_base + NUM_PORT_REGS *
> -				(pdev->usb3_port.port_num - 1));
> +		pdev->usb3_port.regs = (struct cdnsp_port_regs __iomem *)
> +				       (&pdev->op_regs->port_reg_base + NUM_PORT_REGS *
> +					(pdev->usb3_port.port_num - 1));
> +		trace_cdnsp_init("Found  USB 3.x port.");

One More blank space after "Found"

Peter
> +	}
>  
>  	return 0;
>  }
> diff --git a/drivers/usb/cdns3/cdnsp-ring.c b/drivers/usb/cdns3/cdnsp-ring.c
> index 0758f171f73e..715658c981ff 100644
> --- a/drivers/usb/cdns3/cdnsp-ring.c
> +++ b/drivers/usb/cdns3/cdnsp-ring.c
> @@ -259,7 +259,7 @@ static bool cdnsp_room_on_ring(struct cdnsp_device *pdev,
>   */
>  static void cdnsp_force_l0_go(struct cdnsp_device *pdev)
>  {
> -	if (pdev->active_port == &pdev->usb2_port && pdev->gadget.lpm_capable)
> +	if (pdev->active_port != &pdev->usb3_port && pdev->gadget.lpm_capable)
>  		cdnsp_set_link_state(pdev, &pdev->active_port->regs->portsc, XDEV_U0);
>  }
>  
> @@ -763,6 +763,8 @@ static int cdnsp_update_port_id(struct cdnsp_device *pdev, u32 port_id)
>  
>  	if (port_id == pdev->usb2_port.port_num) {
>  		port = &pdev->usb2_port;
> +	} else if (port_id == pdev->eusb_port.port_num) {
> +		port = &pdev->eusb_port;
>  	} else if (port_id == pdev->usb3_port.port_num) {
>  		port  = &pdev->usb3_port;
>  	} else {
> @@ -779,7 +781,8 @@ static int cdnsp_update_port_id(struct cdnsp_device *pdev, u32 port_id)
>  		cdnsp_enable_slot(pdev);
>  	}
>  
> -	if (port_id == pdev->usb2_port.port_num)
> +	if ((pdev->usb2_port.exist && port_id == pdev->usb2_port.port_num) ||
> +	    (pdev->eusb_port.exist && port_id == pdev->eusb_port.port_num))
>  		cdnsp_set_usb2_hardware_lpm(pdev, NULL, 1);
>  	else
>  		writel(PORT_U1_TIMEOUT(1) | PORT_U2_TIMEOUT(1),
> @@ -808,7 +811,7 @@ static void cdnsp_handle_port_status(struct cdnsp_device *pdev,
>  
>  	port_regs = pdev->active_port->regs;
>  
> -	if (port_id == pdev->usb2_port.port_num)
> +	if (port_id == pdev->usb2_port.port_num || port_id == pdev->eusb_port.port_num)
>  		port2 = true;
>  
>  new_event:
> 
> ---
> base-commit: 1c7cc4904160c6fc6377564140062d68a3dc93a0
> change-id: 20260417-eusb2v2_upstream-80c5b29a7bba
> 
> Best regards,
> --  
> Pawel Laszczak <pawell@cadence.com>
> 
> 

-- 

Best regards,
Peter

RE: [PATCH] usb: cdnsp: add support for eUSB2v2 port

[Pawel Laszczak]

>On 26-04-17 10:37:31, Pawel Laszczak via B4 Relay wrote:
>> From: Pawel Laszczak <pawell@cadence.com>
>>
>> The Cadence CDNSP controller optionally supports eUSB2 (embedded USB2)
>> port. While this port type operates logically like high-speed USB 2.0,
>> it utilizes a different physical layer signaling.
>>
>> This patch:
>> - Extends the port detection logic to recognize the eUSB2 protocol.
>> - Tracks the eUSB2 port offset in the cdnsp_device structure.
>> - Ensures that eUSB2 ports are correctly handled during Link State
>>   transitions, specifically forcing L0 when LPM is capable, similar
>>   to standard USB 2.0 ports.
>>
>> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
>
>Pawel, I would like double confirm if you have tested current USB2 and
>USB3 device mode, basically, I think you did it.

Yes, I've tested. 
>
>> ---
>>  drivers/usb/cdns3/cdnsp-gadget.c | 49 ++++++++++++++++++---------
>>  drivers/usb/cdns3/cdnsp-gadget.h |  1 +
>>  drivers/usb/cdns3/cdnsp-mem.c    | 73 +++++++++++++++++++++++++++-----
>--------
>>  drivers/usb/cdns3/cdnsp-ring.c   |  9 +++--
>>  4 files changed, 90 insertions(+), 42 deletions(-)
>>
>> diff --git a/drivers/usb/cdns3/cdnsp-gadget.c b/drivers/usb/cdns3/cdnsp-
>gadget.c
>> index 6b3815f8a6e5..2c71c77e6ec3 100644
>> --- a/drivers/usb/cdns3/cdnsp-gadget.c
>> +++ b/drivers/usb/cdns3/cdnsp-gadget.c
>> @@ -124,20 +124,28 @@ void cdnsp_set_link_state(struct cdnsp_device
>*pdev,
>>  }
>>
>>  static void cdnsp_disable_port(struct cdnsp_device *pdev,
>> -			       __le32 __iomem *port_regs)
>> +			       struct cdnsp_port *port)
>>  {
>> -	u32 temp = cdnsp_port_state_to_neutral(readl(port_regs));
>> +	u32 temp;
>> +
>> +	if (!port->exist)
>> +		return;
>>
>> -	writel(temp | PORT_PED, port_regs);
>> +	temp = cdnsp_port_state_to_neutral(readl(&port->regs->portsc));
>> +	writel(temp | PORT_PED, &port->regs->portsc);
>>  }
>
>Why above changes are added, is it related to this change?

Changed the function argument allows the function to verify the port's
existence before accessing registers that might have been removed.

Although the driver can detect three ports (eusb2, USB2.0, and USB3.0),
either the USB2.0 or the eusb2 port will likely be "removed" from the
controller, depending on the controller configuration.
These two ports (eusb2 and USB2.0) will not work simultaneously
on a single SoC because this two port uses different voltage.

By 'removed', I mean that while the register map remains unchanged,
the underlying logic will be stripped out to reduce the controller size.
Both eUSB2 and USB2.0 ports have their own dedicated register groups.


>>
>>  static void cdnsp_clear_port_change_bit(struct cdnsp_device *pdev,
>> -					__le32 __iomem *port_regs)
>> +					struct cdnsp_port *port)
>>  {
>> -	u32 portsc = readl(port_regs);
>> +	u32 portsc;
>> +
>> +	if (!port->exist)
>> +		return;
>>
>> +	portsc = readl(&port->regs->portsc);
>>  	writel(cdnsp_port_state_to_neutral(portsc) |
>> -	       (portsc & PORT_CHANGE_BITS), port_regs);
>> +	       (portsc & PORT_CHANGE_BITS), &port->regs->portsc);
>>  }
>
>ditto
>
>>
>>  static void cdnsp_set_apb_timeout_value(struct cdnsp_device *pdev)
>> @@ -944,7 +952,7 @@ void cdnsp_set_usb2_hardware_lpm(struct
>cdnsp_device *pdev,
>>  				 struct usb_request *req,
>>  				 int enable)
>>  {
>> -	if (pdev->active_port != &pdev->usb2_port || !pdev-
>>gadget.lpm_capable)
>> +	if (pdev->active_port == &pdev->usb3_port || !pdev-
>>gadget.lpm_capable)
>>  		return;
>>
>>  	trace_cdnsp_lpm(enable);
>> @@ -1310,20 +1318,26 @@ static int cdnsp_run(struct cdnsp_device *pdev,
>>  		break;
>>  	}
>>
>> -	if (speed >= USB_SPEED_SUPER) {
>> +	if (pdev->usb3_port.exist && speed >= USB_SPEED_SUPER) {
>>  		writel(temp, &pdev->port3x_regs->mode_addr);
>>  		cdnsp_set_link_state(pdev, &pdev->usb3_port.regs->portsc,
>>  				     XDEV_RXDETECT);
>>  	} else {
>> -		cdnsp_disable_port(pdev, &pdev->usb3_port.regs->portsc);
>> +		cdnsp_disable_port(pdev, &pdev->usb3_port);
>>  	}
>>
>> -	cdnsp_set_link_state(pdev, &pdev->usb2_port.regs->portsc,
>> -			     XDEV_RXDETECT);
>> +	if (pdev->usb2_port.exist) {
>> +		cdnsp_set_link_state(pdev, &pdev->usb2_port.regs->portsc,
>> +				     XDEV_RXDETECT);
>> +		writel(PORT_REG6_L1_L0_HW_EN | fs_speed, &pdev-
>>port20_regs->port_reg6);
>> +	}
>> +
>> +	if (pdev->eusb_port.exist)
>> +		cdnsp_set_link_state(pdev, &pdev->eusb_port.regs->portsc,
>> +				     XDEV_RXDETECT);
>>
>>  	cdnsp_gadget_ep0_desc.wMaxPacketSize = cpu_to_le16(512);
>>
>> -	writel(PORT_REG6_L1_L0_HW_EN | fs_speed, &pdev->port20_regs-
>>port_reg6);
>>
>>  	ret = cdnsp_start(pdev);
>>  	if (ret) {
>> @@ -1469,8 +1483,10 @@ static void cdnsp_stop(struct cdnsp_device *pdev)
>>  			cdnsp_ep_dequeue(&pdev->eps[0], req);
>>  	}
>>
>> -	cdnsp_disable_port(pdev, &pdev->usb2_port.regs->portsc);
>> -	cdnsp_disable_port(pdev, &pdev->usb3_port.regs->portsc);
>> +	cdnsp_disable_port(pdev, &pdev->usb2_port);
>> +	cdnsp_disable_port(pdev, &pdev->usb3_port);
>> +	cdnsp_disable_port(pdev, &pdev->eusb_port);
>> +
>>  	cdnsp_disable_slot(pdev);
>>  	cdnsp_halt(pdev);
>>
>> @@ -1479,8 +1495,9 @@ static void cdnsp_stop(struct cdnsp_device *pdev)
>>  	temp = readl(&pdev->ir_set->irq_pending);
>>  	writel(IMAN_IE_CLEAR(temp), &pdev->ir_set->irq_pending);
>>
>> -	cdnsp_clear_port_change_bit(pdev, &pdev->usb2_port.regs->portsc);
>> -	cdnsp_clear_port_change_bit(pdev, &pdev->usb3_port.regs->portsc);
>> +	cdnsp_clear_port_change_bit(pdev, &pdev->usb2_port);
>> +	cdnsp_clear_port_change_bit(pdev, &pdev->eusb_port);
>> +	cdnsp_clear_port_change_bit(pdev, &pdev->usb3_port);
>>
>>  	/* Clear interrupt line */
>>  	temp = readl(&pdev->ir_set->irq_pending);
>> diff --git a/drivers/usb/cdns3/cdnsp-gadget.h b/drivers/usb/cdns3/cdnsp-
>gadget.h
>> index a91cca509db0..c44bca348a41 100644
>> --- a/drivers/usb/cdns3/cdnsp-gadget.h
>> +++ b/drivers/usb/cdns3/cdnsp-gadget.h
>> @@ -1474,6 +1474,7 @@ struct cdnsp_device {
>>  	unsigned int link_state;
>>
>>  	struct cdnsp_port usb2_port;
>> +	struct cdnsp_port eusb_port;
>>  	struct cdnsp_port usb3_port;
>>  	struct cdnsp_port *active_port;
>>  	u16 test_mode;
>> diff --git a/drivers/usb/cdns3/cdnsp-mem.c b/drivers/usb/cdns3/cdnsp-mem.c
>> index a2a1b21f2ef8..932071818d1e 100644
>> --- a/drivers/usb/cdns3/cdnsp-mem.c
>> +++ b/drivers/usb/cdns3/cdnsp-mem.c
>> @@ -1088,11 +1088,9 @@ void cdnsp_mem_cleanup(struct cdnsp_device
>*pdev)
>>  			  pdev->dcbaa, pdev->dcbaa->dma);
>>
>>  	pdev->dcbaa = NULL;
>> -
>> -	pdev->usb2_port.exist = 0;
>> -	pdev->usb3_port.exist = 0;
>> -	pdev->usb2_port.port_num = 0;
>> -	pdev->usb3_port.port_num = 0;
>> +	memset(&pdev->usb2_port, 0, sizeof(struct cdnsp_port));
>> +	memset(&pdev->eusb_port, 0, sizeof(struct cdnsp_port));
>> +	memset(&pdev->usb3_port, 0, sizeof(struct cdnsp_port));
>>  	pdev->active_port = NULL;
>>  }
>>
>> @@ -1133,6 +1131,18 @@ static void cdnsp_add_in_port(struct cdnsp_device
>*pdev,
>>  	port_offset = CDNSP_EXT_PORT_OFF(temp);
>>  	port_count = CDNSP_EXT_PORT_COUNT(temp);
>>
>> +	if (port == &pdev->eusb_port) {
>> +		/*
>> +		 * If controller has usb2 + eusb port then eusb is as
>> +		 * second port
>> +		 */
>
>What kinds of topology like below usb2 + eusb?

In the case of eUSB2, both USB2.0 and eUSB2 ports will be visible.
The USB2.0 port appears first. As I mentioned earlier, for eUSB2 SoCs,
the USB2.0 logic will likely be stripped out, but the port will remain
visible in the configuration.

>
>> +		if (port_count == 2)
>> +			port_offset++;
>> +
>> +		if (port_count == 1 && pdev->usb2_port.exist)
>> +			return;
>> +	}
>> +
>>  	trace_cdnsp_port_info(addr, port_offset, port_count, port->maj_rev);
>>
>>  	port->port_num = port_offset;
>> @@ -1152,13 +1162,10 @@ static int cdnsp_setup_port_arrays(struct
>cdnsp_device *pdev)
>>  	base = &pdev->cap_regs->hc_capbase;
>>  	offset = cdnsp_find_next_ext_cap(base, 0,
>>  					 EXT_CAP_CFG_DEV_20PORT_CAP_ID);
>> -	pdev->port20_regs = base + offset;
>> -
>> -	offset = cdnsp_find_next_ext_cap(base, 0, D_XEC_CFG_3XPORT_CAP);
>> -	pdev->port3x_regs =  base + offset;
>> +	if (offset)
>> +		pdev->port20_regs = base + offset;
>>
>>  	offset = 0;
>> -	base = &pdev->cap_regs->hc_capbase;
>>
>>  	/* Driver expects max 2 extended protocol capability. */
>>  	for (i = 0; i < 2; i++) {
>> @@ -1173,26 +1180,46 @@ static int cdnsp_setup_port_arrays(struct
>cdnsp_device *pdev)
>>  			cdnsp_add_in_port(pdev, &pdev->usb3_port,
>>  					  base + offset);
>>
>> -		if (CDNSP_EXT_PORT_MAJOR(temp) == 0x02 &&
>> -		    !pdev->usb2_port.port_num)
>> -			cdnsp_add_in_port(pdev, &pdev->usb2_port,
>> -					  base + offset);
>> +		if (CDNSP_EXT_PORT_MAJOR(temp) == 0x02) {
>> +			if (!pdev->usb2_port.port_num && pdev->port20_regs)
>
>Why "&& pdev->port20_regs" is added?

This additional check for the pdev->port20_regs register group occurs
only for the USB 2.0 port.

>
>> +				cdnsp_add_in_port(pdev, &pdev->usb2_port,
>> +						  base + offset);
>> +
>> +			if (!pdev->eusb_port.port_num)
>> +				cdnsp_add_in_port(pdev, &pdev->eusb_port,
>> +						  base + offset);
>> +		}
>>  	}
>>
>> -	if (!pdev->usb2_port.exist || !pdev->usb3_port.exist) {
>> -		dev_err(pdev->dev, "Error: Only one port detected\n");
>> +	if (!pdev->usb2_port.exist && !pdev->eusb_port.exist &&
>> +	    !pdev->usb3_port.exist) {
>> +		dev_err(pdev->dev, "Error: No port detected\n");
>>  		return -ENODEV;
>>  	}
>>
>> -	trace_cdnsp_init("Found USB 2.0 ports and  USB 3.0 ports.");
>> +	if (pdev->usb2_port.exist) {
>> +		pdev->usb2_port.regs = (struct cdnsp_port_regs __iomem *)
>> +				       (&pdev->op_regs->port_reg_base +
>NUM_PORT_REGS *
>> +					(pdev->usb2_port.port_num - 1));
>> +		trace_cdnsp_init("Found USB 2.0 port.");
>> +	}
>>
>> -	pdev->usb2_port.regs = (struct cdnsp_port_regs __iomem *)
>> -			       (&pdev->op_regs->port_reg_base +
>NUM_PORT_REGS *
>> -				(pdev->usb2_port.port_num - 1));
>> +	if (pdev->eusb_port.exist) {
>> +		pdev->eusb_port.regs = (struct cdnsp_port_regs __iomem *)
>> +				       (&pdev->op_regs->port_reg_base +
>NUM_PORT_REGS *
>> +					(pdev->eusb_port.port_num - 1));
>> +		trace_cdnsp_init("Found eUSB 2.0 port.");
>> +	}
>> +
>> +	if (pdev->usb3_port.exist) {
>> +		offset = cdnsp_find_next_ext_cap(base, 0,
>D_XEC_CFG_3XPORT_CAP);
>> +		pdev->port3x_regs =  base + offset;
>>
>> -	pdev->usb3_port.regs = (struct cdnsp_port_regs __iomem *)
>> -			       (&pdev->op_regs->port_reg_base +
>NUM_PORT_REGS *
>> -				(pdev->usb3_port.port_num - 1));
>> +		pdev->usb3_port.regs = (struct cdnsp_port_regs __iomem *)
>> +				       (&pdev->op_regs->port_reg_base +
>NUM_PORT_REGS *
>> +					(pdev->usb3_port.port_num - 1));
>> +		trace_cdnsp_init("Found  USB 3.x port.");
>
>One More blank space after "Found"
>
>Peter
>> +	}
>>
>>  	return 0;
>>  }
>> diff --git a/drivers/usb/cdns3/cdnsp-ring.c b/drivers/usb/cdns3/cdnsp-ring.c
>> index 0758f171f73e..715658c981ff 100644
>> --- a/drivers/usb/cdns3/cdnsp-ring.c
>> +++ b/drivers/usb/cdns3/cdnsp-ring.c
>> @@ -259,7 +259,7 @@ static bool cdnsp_room_on_ring(struct cdnsp_device
>*pdev,
>>   */
>>  static void cdnsp_force_l0_go(struct cdnsp_device *pdev)
>>  {
>> -	if (pdev->active_port == &pdev->usb2_port && pdev-
>>gadget.lpm_capable)
>> +	if (pdev->active_port != &pdev->usb3_port && pdev-
>>gadget.lpm_capable)
>>  		cdnsp_set_link_state(pdev, &pdev->active_port->regs->portsc,
>XDEV_U0);
>>  }
>>
>> @@ -763,6 +763,8 @@ static int cdnsp_update_port_id(struct cdnsp_device
>*pdev, u32 port_id)
>>
>>  	if (port_id == pdev->usb2_port.port_num) {
>>  		port = &pdev->usb2_port;
>> +	} else if (port_id == pdev->eusb_port.port_num) {
>> +		port = &pdev->eusb_port;
>>  	} else if (port_id == pdev->usb3_port.port_num) {
>>  		port  = &pdev->usb3_port;
>>  	} else {
>> @@ -779,7 +781,8 @@ static int cdnsp_update_port_id(struct cdnsp_device
>*pdev, u32 port_id)
>>  		cdnsp_enable_slot(pdev);
>>  	}
>>
>> -	if (port_id == pdev->usb2_port.port_num)
>> +	if ((pdev->usb2_port.exist && port_id == pdev->usb2_port.port_num) ||
>> +	    (pdev->eusb_port.exist && port_id == pdev->eusb_port.port_num))
>>  		cdnsp_set_usb2_hardware_lpm(pdev, NULL, 1);
>>  	else
>>  		writel(PORT_U1_TIMEOUT(1) | PORT_U2_TIMEOUT(1),
>> @@ -808,7 +811,7 @@ static void cdnsp_handle_port_status(struct
>cdnsp_device *pdev,
>>
>>  	port_regs = pdev->active_port->regs;
>>
>> -	if (port_id == pdev->usb2_port.port_num)
>> +	if (port_id == pdev->usb2_port.port_num || port_id == pdev-
>>eusb_port.port_num)
>>  		port2 = true;
>>
>>  new_event:
>>
>> ---
>> base-commit: 1c7cc4904160c6fc6377564140062d68a3dc93a0
>> change-id: 20260417-eusb2v2_upstream-80c5b29a7bba
>>
>> Best regards,
>> --
>> Pawel Laszczak <pawell@cadence.com>
>>
>>
>
>--
>
>Best regards,
>Peter

Re: [PATCH] usb: cdnsp: add support for eUSB2v2 port

[Peter Chen (CIX)]

On 26-04-20 06:31:59, Pawel Laszczak wrote:
> >On 26-04-17 10:37:31, Pawel Laszczak via B4 Relay wrote:
> >> From: Pawel Laszczak <pawell@cadence.com>
> >>
> >> The Cadence CDNSP controller optionally supports eUSB2 (embedded USB2)
> >> port. While this port type operates logically like high-speed USB 2.0,
> >> it utilizes a different physical layer signaling.
> >>
> >> This patch:
> >> - Extends the port detection logic to recognize the eUSB2 protocol.
> >> - Tracks the eUSB2 port offset in the cdnsp_device structure.
> >> - Ensures that eUSB2 ports are correctly handled during Link State
> >>   transitions, specifically forcing L0 when LPM is capable, similar
> >>   to standard USB 2.0 ports.
> >>
> >> Signed-off-by: Pawel Laszczak <pawell@cadence.com>

Except one extra blank space, others are okay for me.

Acked-by: Peter Chen <peter.chen@kernel.org>

Peter
> >
> >Pawel, I would like double confirm if you have tested current USB2 and
> >USB3 device mode, basically, I think you did it.
> 
> Yes, I've tested. 
> >
> >> ---
> >>  drivers/usb/cdns3/cdnsp-gadget.c | 49 ++++++++++++++++++---------
> >>  drivers/usb/cdns3/cdnsp-gadget.h |  1 +
> >>  drivers/usb/cdns3/cdnsp-mem.c    | 73 +++++++++++++++++++++++++++-----
> >--------
> >>  drivers/usb/cdns3/cdnsp-ring.c   |  9 +++--
> >>  4 files changed, 90 insertions(+), 42 deletions(-)
> >>
> >> diff --git a/drivers/usb/cdns3/cdnsp-gadget.c b/drivers/usb/cdns3/cdnsp-
> >gadget.c
> >> index 6b3815f8a6e5..2c71c77e6ec3 100644
> >> --- a/drivers/usb/cdns3/cdnsp-gadget.c
> >> +++ b/drivers/usb/cdns3/cdnsp-gadget.c
> >> @@ -124,20 +124,28 @@ void cdnsp_set_link_state(struct cdnsp_device
> >*pdev,
> >>  }
> >>
> >>  static void cdnsp_disable_port(struct cdnsp_device *pdev,
> >> -			       __le32 __iomem *port_regs)
> >> +			       struct cdnsp_port *port)
> >>  {
> >> -	u32 temp = cdnsp_port_state_to_neutral(readl(port_regs));
> >> +	u32 temp;
> >> +
> >> +	if (!port->exist)
> >> +		return;
> >>
> >> -	writel(temp | PORT_PED, port_regs);
> >> +	temp = cdnsp_port_state_to_neutral(readl(&port->regs->portsc));
> >> +	writel(temp | PORT_PED, &port->regs->portsc);
> >>  }
> >
> >Why above changes are added, is it related to this change?
> 
> Changed the function argument allows the function to verify the port's
> existence before accessing registers that might have been removed.
> 
> Although the driver can detect three ports (eusb2, USB2.0, and USB3.0),
> either the USB2.0 or the eusb2 port will likely be "removed" from the
> controller, depending on the controller configuration.
> These two ports (eusb2 and USB2.0) will not work simultaneously
> on a single SoC because this two port uses different voltage.
> 
> By 'removed', I mean that while the register map remains unchanged,
> the underlying logic will be stripped out to reduce the controller size.
> Both eUSB2 and USB2.0 ports have their own dedicated register groups.
> 
> 
> >>
> >> @@ -1133,6 +1131,18 @@ static void cdnsp_add_in_port(struct cdnsp_device
> >*pdev,
> >>  	port_offset = CDNSP_EXT_PORT_OFF(temp);
> >>  	port_count = CDNSP_EXT_PORT_COUNT(temp);
> >>
> >> +	if (port == &pdev->eusb_port) {
> >> +		/*
> >> +		 * If controller has usb2 + eusb port then eusb is as
> >> +		 * second port
> >> +		 */
> >
> >What kinds of topology like below usb2 + eusb?
> 
> In the case of eUSB2, both USB2.0 and eUSB2 ports will be visible.
> The USB2.0 port appears first. As I mentioned earlier, for eUSB2 SoCs,
> the USB2.0 logic will likely be stripped out, but the port will remain
> visible in the configuration.
> 
> >
> >> +		if (port_count == 2)
> >> +			port_offset++;
> >> +
> >> +		if (port_count == 1 && pdev->usb2_port.exist)
> >> +			return;
> >> +	}
> >> +
> >>  	trace_cdnsp_port_info(addr, port_offset, port_count, port->maj_rev);
> >>
> >>  	port->port_num = port_offset;
> >> @@ -1152,13 +1162,10 @@ static int cdnsp_setup_port_arrays(struct
> >cdnsp_device *pdev)
> >>  	base = &pdev->cap_regs->hc_capbase;
> >>  	offset = cdnsp_find_next_ext_cap(base, 0,
> >>  					 EXT_CAP_CFG_DEV_20PORT_CAP_ID);
> >> -	pdev->port20_regs = base + offset;
> >> -
> >> -	offset = cdnsp_find_next_ext_cap(base, 0, D_XEC_CFG_3XPORT_CAP);
> >> -	pdev->port3x_regs =  base + offset;
> >> +	if (offset)
> >> +		pdev->port20_regs = base + offset;
> >>
> >>  	offset = 0;
> >> -	base = &pdev->cap_regs->hc_capbase;
> >>
> >>  	/* Driver expects max 2 extended protocol capability. */
> >>  	for (i = 0; i < 2; i++) {
> >> @@ -1173,26 +1180,46 @@ static int cdnsp_setup_port_arrays(struct
> >cdnsp_device *pdev)
> >>  			cdnsp_add_in_port(pdev, &pdev->usb3_port,
> >>  					  base + offset);
> >>
> >> -		if (CDNSP_EXT_PORT_MAJOR(temp) == 0x02 &&
> >> -		    !pdev->usb2_port.port_num)
> >> -			cdnsp_add_in_port(pdev, &pdev->usb2_port,
> >> -					  base + offset);
> >> +		if (CDNSP_EXT_PORT_MAJOR(temp) == 0x02) {
> >> +			if (!pdev->usb2_port.port_num && pdev->port20_regs)
> >
> >Why "&& pdev->port20_regs" is added?
> 
> This additional check for the pdev->port20_regs register group occurs
> only for the USB 2.0 port.
> 
> >
> >> +				cdnsp_add_in_port(pdev, &pdev->usb2_port,
> >> +						  base + offset);
> >> +
> >> +			if (!pdev->eusb_port.port_num)
> >> +				cdnsp_add_in_port(pdev, &pdev->eusb_port,
> >> +						  base + offset);
> >> +		}
> >>  	}
> >>
> >> -	if (!pdev->usb2_port.exist || !pdev->usb3_port.exist) {
> >> -		dev_err(pdev->dev, "Error: Only one port detected\n");
> >> +	if (!pdev->usb2_port.exist && !pdev->eusb_port.exist &&
> >> +	    !pdev->usb3_port.exist) {
> >> +		dev_err(pdev->dev, "Error: No port detected\n");
> >>  		return -ENODEV;
> >>  	}
> >>
> >> -	trace_cdnsp_init("Found USB 2.0 ports and  USB 3.0 ports.");
> >> +	if (pdev->usb2_port.exist) {
> >> +		pdev->usb2_port.regs = (struct cdnsp_port_regs __iomem *)
> >> +				       (&pdev->op_regs->port_reg_base +
> >NUM_PORT_REGS *
> >> +					(pdev->usb2_port.port_num - 1));
> >> +		trace_cdnsp_init("Found USB 2.0 port.");
> >> +	}
> >>
> >> -	pdev->usb2_port.regs = (struct cdnsp_port_regs __iomem *)
> >> -			       (&pdev->op_regs->port_reg_base +
> >NUM_PORT_REGS *
> >> -				(pdev->usb2_port.port_num - 1));
> >> +	if (pdev->eusb_port.exist) {
> >> +		pdev->eusb_port.regs = (struct cdnsp_port_regs __iomem *)
> >> +				       (&pdev->op_regs->port_reg_base +
> >NUM_PORT_REGS *
> >> +					(pdev->eusb_port.port_num - 1));
> >> +		trace_cdnsp_init("Found eUSB 2.0 port.");
> >> +	}
> >> +
> >> +	if (pdev->usb3_port.exist) {
> >> +		offset = cdnsp_find_next_ext_cap(base, 0,
> >D_XEC_CFG_3XPORT_CAP);
> >> +		pdev->port3x_regs =  base + offset;
> >>
> >> -	pdev->usb3_port.regs = (struct cdnsp_port_regs __iomem *)
> >> -			       (&pdev->op_regs->port_reg_base +
> >NUM_PORT_REGS *
> >> -				(pdev->usb3_port.port_num - 1));
> >> +		pdev->usb3_port.regs = (struct cdnsp_port_regs __iomem *)
> >> +				       (&pdev->op_regs->port_reg_base +
> >NUM_PORT_REGS *
> >> +					(pdev->usb3_port.port_num - 1));
> >> +		trace_cdnsp_init("Found  USB 3.x port.");
> >
> >One More blank space after "Found"
> >
> >Peter
> >> +	}
> >>
> >>  	return 0;
> >>  }
> >> diff --git a/drivers/usb/cdns3/cdnsp-ring.c b/drivers/usb/cdns3/cdnsp-ring.c
> >> index 0758f171f73e..715658c981ff 100644
> >> --- a/drivers/usb/cdns3/cdnsp-ring.c
> >> +++ b/drivers/usb/cdns3/cdnsp-ring.c
> >> @@ -259,7 +259,7 @@ static bool cdnsp_room_on_ring(struct cdnsp_device
> >*pdev,
> >>   */
> >>  static void cdnsp_force_l0_go(struct cdnsp_device *pdev)
> >>  {
> >> -	if (pdev->active_port == &pdev->usb2_port && pdev-
> >>gadget.lpm_capable)
> >> +	if (pdev->active_port != &pdev->usb3_port && pdev-
> >>gadget.lpm_capable)
> >>  		cdnsp_set_link_state(pdev, &pdev->active_port->regs->portsc,
> >XDEV_U0);
> >>  }
> >>
> >> @@ -763,6 +763,8 @@ static int cdnsp_update_port_id(struct cdnsp_device
> >*pdev, u32 port_id)
> >>
> >>  	if (port_id == pdev->usb2_port.port_num) {
> >>  		port = &pdev->usb2_port;
> >> +	} else if (port_id == pdev->eusb_port.port_num) {
> >> +		port = &pdev->eusb_port;
> >>  	} else if (port_id == pdev->usb3_port.port_num) {
> >>  		port  = &pdev->usb3_port;
> >>  	} else {
> >> @@ -779,7 +781,8 @@ static int cdnsp_update_port_id(struct cdnsp_device
> >*pdev, u32 port_id)
> >>  		cdnsp_enable_slot(pdev);
> >>  	}
> >>
> >> -	if (port_id == pdev->usb2_port.port_num)
> >> +	if ((pdev->usb2_port.exist && port_id == pdev->usb2_port.port_num) ||
> >> +	    (pdev->eusb_port.exist && port_id == pdev->eusb_port.port_num))
> >>  		cdnsp_set_usb2_hardware_lpm(pdev, NULL, 1);
> >>  	else
> >>  		writel(PORT_U1_TIMEOUT(1) | PORT_U2_TIMEOUT(1),
> >> @@ -808,7 +811,7 @@ static void cdnsp_handle_port_status(struct
> >cdnsp_device *pdev,
> >>
> >>  	port_regs = pdev->active_port->regs;
> >>
> >> -	if (port_id == pdev->usb2_port.port_num)
> >> +	if (port_id == pdev->usb2_port.port_num || port_id == pdev-
> >>eusb_port.port_num)
> >>  		port2 = true;
> >>
> >>  new_event:
> >>
> >> ---
> >> base-commit: 1c7cc4904160c6fc6377564140062d68a3dc93a0
> >> change-id: 20260417-eusb2v2_upstream-80c5b29a7bba
> >>
> >> Best regards,
> >> --
> >> Pawel Laszczak <pawell@cadence.com>
> >>
> >>
> >
> >--
> >
> >Best regards,
> >Peter

-- 

Best regards,
Peter